I ve done the Graham Common Lisp Chapter 5 Exercise 5, which requires a function that takes an object X and a vector V, and returns a list of all the objects that immediately precede X in V.
It works like:
> (preceders #a "abracadabra")
(#c #d #r)
I have done the recursive version:
(defun preceders (obj vec &optional (result nil) &key (startt 0))
(let ((l (length vec)))
(cond ((null (position obj vec :start startt :end l)) result)
((= (position obj vec :start startt :end l) 0)
(preceders obj vec result
:startt (1+ (position obj vec :start startt :end l))))
((> (position obj vec :start startt :end l) 0)
(cons (elt vec (1- (position obj vec :start startt :end l)))
(preceders obj vec result
:startt (1+ (position obj vec
:start startt
:end l))))))))
It works correctly, but my teachers gives me the following critique:
"This calls length repeatedly. Not so bad with vectors, but still unnecessary. More efficient and more flexible (for the user) code is to define this like other sequence processing functions. Use :start and :end keyword parameters, the way the other sequence functions do, with the same default initial values. length should need to be called at most once."
I am consulting the Common Lisp textbook and google, but there seem to be of little help on this bit: I don t know what he means by "using :start and :end keyword parameters", and I have no clue of how to "call length just once". I would be grateful if you guys could give me some idea how on to refine my code to meet the requirement that my teacher posted.
UPDATE:
Now I have come up with the following code:
(defun preceders (obj vec
&optional (result nil)
&key (start 0) (end (length vec)) (test # eql))
(let ((pos (position obj vec :start start :end end :test test)))
(cond ((null pos) result)
((zerop pos) (preceders obj vec result
:start (1+ pos) :end end :test test))
(t (preceders obj vec (cons (elt vec (1- pos)) result)
:start (1+ pos) :end end :test test)))))
I get this critique:
"When you have a complex recursive call that is repeated identically in more than one branch, it s often simpler to do the call first, save it in a local variable, and then use the variable in a much simpler IF or COND."
Also,for my iterative version of the function:
(defun preceders (obj vec)
(do ((i 0 (1+ i))
(r nil (if (and (eql (aref vec i) obj)
(> i 0))
(cons (aref vec (1- i)) r)
r)))
((eql i (length vec)) (reverse r))))
I get the critique
"Start the DO at a better point and remove the repeated > 0 test"