Well, for one, assuming that new-lst is a variable defined outside of the function, you're setting it to '() every time you call the function, so you never actually build a full list. the second problem is that you're using cons. Cons is a non-destructive function, meaning that (cons (car lst) new-lst) won't actually alter new-lst, but instead produce a new list, which is immediately discarded. There are 2 ways to get the effect you want. You either use push or (setq new-lst (cons (car lst) new-lst), both of which essentially do the same. If you do this you'll also note that you're building the list in reverse, so you'll have to use reverse (or it's destructive brother nreverse) to get it back in the right order. Note that these also do not alter new-lst, but return a new list that is a reversed variant of new-lst.
Now, to solve the first problem, you either need to make it so that new-lst is shared between the different instances of numbers (a so-called closure, closing over an external value) using
Code: Select all
(let ((new-lst (list)))
(defun numbers (lst) ...)
but that has the problem that you need to null the list by hand (or using a wrapper function) every time you call numbers. A better way would be to use labels to define a recursive function local to numbers and use that for recursion:
Code: Select all
(defun numbers (lst)
(let ((new-lst (list)))
(labels ((list-walker (list) ....))
(list-walker lst)
(nreverse new-lst)) ;assuming you're using push or cons to build your list you need to reverse it when you're done
However, while using a closure is a good idea in this case, it misses a central point of recursion and that is how to handle return values. We could modify your specification as following:
1.) If the car of our list is a number, make a list of all numbers in the remaining list and cons them together.
2.) else, if there are still elements in the list beyond this one, return all numbers in that list
3.) else, return nil.
This takes advantage of the fact that consing to nil will result in a fresh list consisting of 1 element, the thing you consed to nil. I'll let you figure out the resulting definition yourself.
On a side note, don't use setq to introduce new variables, it's not common lisp compliant. To see what exactly happens if I run your function I would have to track down an implementation that does support this idiom instead of using the one I usually use. Used defvar to introduce globally-bound, special* variables or let to introduce local, lexical* variables.
*The difference between lexical and special is best illustrated with an example:
Code: Select all
(defvar *a* 3) ;*a* is now defined as special
(defun test-a () *a*)
(test-a) ;This yields 3
(let ((*a* 5)) ;Even though we use let, *a* was already special and will not become lexical
(test-a) ;Because *a* is special, we can shadow it at run-time with let, and the effect will propagate to functions defined earlier
(let ((b 4)) ;b is now lexical
(defun test-b () b)) ;Since b is lexical, it's value will be locked in when we compile this function
(test-b) ;this yields 4
(let ((b 6))
(test-b) ;this also yields 4, because due to b's lexical nature, we cannot shadow it any more.
In essence, special variables behave like global variables in other languages, while lexical variables behave like local variables. This isn't perfectly correct of course, defining 2 functions that refer to the same lexical variable and you have a "local" variable that isn't local to either function, but it helps to think like that. Also if you really need to, you can use (declare (special variable)) directly after a let-form's variable definition to tell the compiler that variable is special, which allows shadowing, but prevents outside access since there is no global symbol associated with the variable, so you have a special local variable then.