Pushing new elements into a new empty list using Lisp

Question

The below function tries to update *features-list* With the element feature. feature-list is defined as a global variable.

When I run the function against an empty *feature-list*,I get an error Message

The object is a CONDITION of type TYPE-ERROR.
DATUM: NIL
EXPECTED-TYPE: CONS

However, when I initialize the *feature-list*, the function executes properly.

(defun update-feature-list (feature)
  (let ((feature-index))
    (setq feature-index (position feature *features-list*))
    (cond ((equal feature-index nil) 
           (push feature (cdr (last *features-list*)))
           (setq feature-index (position feature *features-list*))))))

Please fix your code - indentation and parentheses. Right now is it unreadable. — sds, Sep 27 '16 at 15:20
See [this comment](http://stackoverflow.com/questions/34793011/push-doesnt-modify-the-list-being-a-function-argument#comment57330915_34793011). — Joshua Taylor, Sep 27 '16 at 17:51

score 3 · Answer 1 · edited Sep 27 '16 at 15:35

Use standard formatting. See http://www.gigamonkeys.com/book/syntax-and-semantics.html#formatting-lisp-code.

(defun update-feature-list (feature)
  (let ((feature-index))
    (setq feature-index (position feature *features-list*))
    (cond ((equal feature-index nil)
           ;;case 1 If feature index ==nil ;;we need to add the feature 
           (push feature (cdr (last *features-list*)))
           (setq feature-index (position feature *features-list*))))))

A cond with one clause is meaningless. I guess you meant when, and that you want to update the index in any case, i. e. outside of the conditional.

(defun update-feature-list (feature)
  (let ((feature-index))
    (setq feature-index (position feature *features-list*))
    (when (equal feature-index nil)
      (push feature (cdr (last *features-list*))))
    (setq feature-index (position feature *features-list*))))

You do not need to set a local variable just to return it:

(defun update-feature-list (feature)
  (let ((feature-index))
    (setq feature-index (position feature *features-list*))
    (when (equal feature-index nil)
      (push feature (cdr (last *features-list*))))
    (position feature *features-list*)))

You can create the binding directly in the let head:

(defun update-feature-list (feature)
  (let ((feature-index (position feature *features-list*)))
    (when (equal feature-index nil)
      (push feature (cdr (last *features-list*))))
    (position feature *features-list*)))

Instead of checking for equal … nil, use null:

(defun update-feature-list (feature)
  (let ((feature-index (position feature *features-list*)))
    (when (null feature-index)
      (push feature (cdr (last *features-list*))))
    (position feature *features-list*)))

You can inline that variable:

(defun update-feature-list (feature)
  (when (null (position feature *features-list*))
    (push feature (cdr (last *features-list*))))
  (position feature *features-list*))

Instead of null … position, use not … member:

(defun update-feature-list (feature)
  (when (not (member feature *features-list*))
    (push feature (cdr (last *features-list*))))
  (position feature *features-list*))

The cdr of the last cons in a list is not a place that you would want to push things onto. I guess that you want to append, but in most cases, you should instead push to the front of the list, which is much more efficient. There is also pushnew for exactly this. Returning the new position does not make much sense now, however:

(defun update-feature-list (feature)
  (pushnew feature *features-list*)
  (position feature *features-list*))

If you really need this order and the positions, use adjustable vectors instead:

(defvar *features-list* (make-array 10
                                    :adjustable t
                                    :fill-pointer 0))

(defun add-feature (feature)
  (or (position feature *features-list*)
      (vector-push-extend feature *features-list*))) ; v-p-e returns the index

thanks...however, i do need this exact order of elements..so i cannot push in the beginning of the list...i tried the above code corrections..With push as well as append..and both have the same behavior..error when the list is empty, nd proper behavior when intialized at least With one element.. — ijuio, Sep 27 '16 at 16:16
so it turns out when i use append insteaad of push..and initialize the list to (defparameter *features_list* nil) instead of what i used to initialize to as (defparameter *featuteres_list* '()) it workd properly.. While using the push, the error persists as long as the list is empty independantly of how it is initialized.. for me, i am happy using the append for now for this..but if someone can explain why push behaves this way on empty lists, it would be great — ijuio, Sep 27 '16 at 16:25
@ijuio: `(push 'foo *list*)` is the same as `(setf *list* (cons 'foo *list*))`. `(list 'a 'b 'c)` is the same as `(cons 'a (cons 'b (cons 'c nil)))`. That is the way lists work in Lisp: they are a cons chain. When your variable is bound to a list, it actually refers to the "first" or "outermost" cons cell. `Last` returns the last or "innermost" cons cell. `Nil` is not a cons but an atom, and it is also used as the empty list. — Svante, Sep 27 '16 at 23:39

score 1 · Accepted Answer · edited Sep 28 '16 at 06:06

Solution provided by this comment from Joshua Taylor.

Your approach to "push some val to a list inside a function" approach doesn't work; what happens if the original list is empty? You can't modify the car or cdr of the empty list; it doesn't have those. That said, if you do want to use the approach you described, you might do it a little more clearly with (push newvalue (rest list)) (rotatef (first list) (second list)). (That's certainly not the only option, though.)

Pushing new elements into a new empty list using Lisp

2 Answers2