views:

812

answers:

7

I have a function that gets x(a value) and xs(a list) and removes all values that are bigger than x from the list. Well it doesn't work, can you tell me why?

(defun biggerElems(x xs) 
  (let ((xst))
    (dolist (elem xs)
      (if (> x elem)
          (setf xst (remove elem xs))))
    xst))
+5  A: 

I think it's this line that's not right:

(setf xst (remove elem xs))))

The first argument to setf is the place, followed by the value. It looks like you have it backwards (and xst is either nil or uninitialized).

You might find it easier to do this:

(defun biggerElems (x xs)
  (remove-if (lambda (item) (> item x)) xs))
Ben Collins
+1  A: 

It worked like that:

(defun filterBig (x xs)
  (remove-if (lambda (item) (> item x)) xs))

What was the '#' for? It didn't compile with it.

Alexander Stolz
A: 

What was the '#' for? It didn't compile with it.

Typo. Normally you refer to functions with #' (like (remove-if #'oddp list)), but when I was editing, I forgot to remove the '#'.

Ben Collins
+1  A: 

If you want to do this the Lisp Way, you could use recursion to return the new list:

(defun biggerElems (x xs)
  (cond ((null xs) NIL)
        ((< x (car xs))
         (biggerElems x (cdr xs)))
       (t
        (cons (car xs) (biggerElems x (cdr xs))))))

@Luís Oliveira

This solution is to contrast with the one posted in the question. Had we needed to do something slightly more complicated it's important to be grounded in the recursive approach to manipulating lists.

Kyle Cronin
Just because you CAN use recursion in Lisp, doesn't mean it's the "Lisp Way". There are several mechanisms for doing this in Lisp (as seen elsewhere here), and all of them are idiomatic. The recursive method is simply the most primitive.
Will Hartung
A: 

@Ben: you can edit your post and fix it you know...

@Kyle: recursion is not "the" Lisp way. Ben's solution is much more clear and concise.

Luís Oliveira
+4  A: 

Most concise AFAIK:

(defun bigger-elements (x xs) (remove x xs :test #'<))

returning a fresh list, it removes all elements y from xs for which

(< y x)

or using the famous LOOP:

(defun bigger-elements-2 (x xs) 
  (loop for e in xs
        unless (< e x)
        collect e))
Bart
+1  A: 

@Ben: It isn't the setf call that is wrong--the problem is that he is not updating xs.

ie: xst is being set to xs with the element removed, but xs is not being updated. If a second element is to be removed, xst will have the first back in it.

you would need to bind xst to xs, and replace the xs in the remove call with xst. This would then remove all elements x is bigger than. ie:

(defun biggerElems(x xs)
  (let ((xst xs))
    (dolist (elem xs)
      (when (> x elem)
        (setf xst (remove elem xst))))
    xst))

It might be slightly faster to set xst to (copy-list xs) and then use delete instead of remove (delete is destructive... depending on your implementation, it may be faster than remove. Since you are calling this multiple times, you may get better performance copying the list once and destructively deleting from it).

Alternatively:

(defun bigger-elems (x xs) ; I prefer hyphen separated to camelCase... to each his own
  (loop for elem in xs when (<= x elem) collect elem))

Looking back over your original post, it is a little confusing... you say you remove all elements bigger than x, but your code looks like it is attempting to remove all elements x is bigger than. The solutions I wrote return all elements bigger than x (ie: remove all elements x is bigger than).

Morikal