Another Lisp function refinement

Question

I've completed the Graham's exercise Chapter 5.8,and my code is:

(defun max-min (vec &key (start 0) (end (length vec)))
  (cond 
    ((eql start (1- end)) (values (elt vec start) (elt vec (1- end))))
    ((zerop end) (values nil nil))
    (t 
       (multiple-value-bind (x y) (max-min vec :start (1+ start) :end end)
       (let* ((maxx x)(minn y))
         (values (max maxx (elt vec start)) (min minn (elt vec start))))))))

You don't need to worry about the details, basically it just returns the max and min of a given vector in a "value" form.
I use the above recursion to solve the problem, but my teachers marked my function as "almost done" with such critique:

"If a function takes start and end, then length is neither needed nor correct. Length could be > 0 but it's whether start < end or not that matters. Testing end all by itself is not relevant at all."

I am not very clear at this point, I tried getting rid of the (length vec) default value for "end", but then the default value for end becomes nil.
We have clear instrution that "length" should at most be called once.
Could you please give me some hint on this? Thanks.

Answer 1

I don't know some of the lisp features you're using, but it looks to me very much like your teacher is incorrect.

"If a function takes start and end, then length is neither needed nor correct. Length could be 0 but it's whether start < end or not that matters. Testing end all by itself is not relevant at all."

It looks to me like length is the default value for end, and that you are only testing end, not testing length at all... His complaint already appears to be addressed.

That said, there are a other things to complain about in that code. For example, what happens if you call max-min with end < start? It looks to me like you'll recurse by incrementing start, until you run out of vector, then you'll get some kind of exception.

Answer 2

Your lambda list is OK. The problem is the base case: (zerop end) should be modified so that you also get a sensible result if called like (min-max myvec :start 5 :end 3).

The next critique is about these two lines:

   (multiple-value-bind (x y) (max-min vec :start (1+ start) :end end)
   (let* ((maxx x) (minn y))
     ;; ...

If you want the results of the recursive call to be named maxx and minn, why don't you name them like that directly?

   (multiple-value-bind (maxx minn) (max-min vec :start (1+ start) :end end)
     ;; ...

By the way, you can call them max and min (there are separate namespaces for variables and functions), or max-of-rest and min-of-rest (to be more descriptive).