Search code examples
common-lispscalingslimeaquamacs

a question on common lisp


I'm getting crazy with a small problem here, I keep getting an error and I cant seem to figure out why, the code is supposed to change the range of a list, so if we give it a list with values (1 2 3 4) and we want to change the range in 11 to fourteen the result would be (11 12 13 14) the problem is that the last function called scale-list will give back an error saying:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)

anybody has a clue why? I use aquamacs as an editor thanks in advance

;;finds minimum in a list
(defun minimum (list)
  (car (sort list #'<)))

;;finds maximum in a list
(defun maximum (list)
  (car (sort list #'>)))

;;calculates the range of a list
(defun range (list)
  (- (maximum list) (minimum list)))

;;scales one value to another range
(defun scale-value (list low high n)
   (+ (/ (* (- (nth (- n 1) list)
               (minimum list))
            (- high low))
         (range list))
      low))


;;is supposed to scale the whole list to another range
(defun scale-list (list low high n)
  (unless (= n 0)
   (cons (scale-value list low high n)
         (scale-list list low high (- n 1)))))

(scale-list '(1 2 3 4) 21 24 4)

Solution

  • The definitions of maximum and minimum need to be improved. SORT is destructive. It is also wrong to call SORT with a literal constant like '(1 2 3 4) - again, SORT is destructive.

    Better definitions:

    (defun minimum (list)
      (reduce #'min list))
    
    (defun maximum (list)
      (reduce #'max list))
    

    A more efficient definition of range:

    (defun range (list)
      (loop for e in list
            maximize e into max
            minimize e into min
            finally (return (- max min))))
    

    SCALE-LIST and SCALE-VALUE are also not Lisp-like. If you call NTH like this in a recursive function then something is wrong. You should recurse over the list, not the index. SCALE-VALUE calls RANGE and MINIMUM for each call. Why?

    Check this variant:

    ;;scales one value to another range
    (defun scale-value (item low high min range)
       (+ (/ (* (- item min)
                (- high low))
             range)
          low))
    
    ;;is supposed to scale the whole list to another range
    (defun scale-list (list low high)
      (let ((min (minimum list))
            (range (range list)))
        (labels ((scale-list-aux (list)
                   (when list
                     (cons (scale-value (first list) low high min range)
                           (scale-list-aux (rest list))))))
          (scale-list-aux list))))
    
    (scale-list '(1 2 3 4) 21 24)
    

    What can you improve more? For example I would get rid of the recursion and replace it with MAPCAR.