CODEWITHSUNDEEP
functionrecursionlispcommon-lisphelper
Ignacious
Ignacious

Reputation: 119

LISP, cond returning nil everytime

Why does this always return nil when its obviously adding it to lst???

Please help!

Thank You! 

CL-USER 1 : 1 > (defun constants-aux (form lst)
                  (cond ((null form) lst)
                        ((eq (car form) 'implies) (constants-aux (cdr form) lst))
                        ((eq (car form) 'not) (constants-aux (cdr form) lst))
                        ((eq (car form) 'and) (constants-aux (cdr form) lst))
                        ((eq (car form) 'or) (constants-aux (cdr form) lst))
                        ((atom (car form)) (print (car form)) (cons (car form) (list lst)) (delete-duplicates lst) (constants-aux (cdr form) lst))
                        (T (constants-aux (car form) lst) (constants-aux (cdr form) lst))))
CONSTANTS-AUX

CL-USER 2 : 1 > (defun constants (form)
                   (constants-aux form nil))
CONSTANTS

CL-USER 3 : 1 > constants '(IMPLIES (NOT Q) (IMPLIES Q P))

Q 
Q 
P 
NIL

Upvotes: 0

Views: 637

Answers (2)

Rainer Joswig
Rainer Joswig

Reputation: 139251

The first thing is to proper format your code. Example:

(defun constants-aux (form lst)
  (cond ((null form) lst)
        ((eq (car form) 'implies) (constants-aux (cdr form) lst))
        ((eq (car form) 'not)     (constants-aux (cdr form) lst))
        ((eq (car form) 'and)     (constants-aux (cdr form) lst))
        ((eq (car form) 'or)      (constants-aux (cdr form) lst))
        ((atom (car form))
         (print (car form))
         (cons (car form) (list lst))
         (delete-duplicates lst)
         (constants-aux (cdr form) lst))
        (T
         (constants-aux (car form) lst)
         (constants-aux (cdr form) lst))))

Then you can simplify your code:

(defun constants-aux (form list)
  (cond ((null form) list)
        ((member (car form) '(implies not and or))
         (constants-aux (cdr form) list))
        ((atom (car form))
         (constants-aux (cdr form)
                        (adjoin (car form) list)))
        (T
         (constants-aux (car form) list)
         (constants-aux (cdr form) list))))

Then we may get rid of car and cdr.

(defun constants-aux (form list)
  (if (consp form)
      (destructuring-bind (head . tail)
          form
        (cond ((member head '(implies not and or))
               (constants-aux tail list))
              ((atom head)
               (constants-aux tail (adjoin head list)))
              (T
               (constants-aux head list)
               (constants-aux tail list))))
    list))

Then you can work a bit on the logic and make sure results are not ignored, make sure the adjoin thing is correct, and so on...

Upvotes: 2

sheikh_anton
sheikh_anton

Reputation: 3452

You doing it wrong in so many ways.

1. - Why create -aux function, when you can just use optional arguments?

(defun constants (form &optional lst)
  (cond
    ((null form) lst) ...

2. - You don't need so many similar branches, you can write:

((find (car form) '(implies not and or))
 (constants (cdr form) lst))

3. - delete-duplicates MAY modify your list, but don't think it have to, and even if it will, the modification it makes not what you wanted. You should use result of it. I think you got even a STYLE-WARNING, in SBCL it looks like this for your code:

; caught STYLE-WARNING:
;   The return value of DELETE-DUPLICATES should not be discarded.

Read warnings, it helps.

I don't understand what result are you expecting, so I can't modify your function to work properly, but I'll try to show you where problem(s) is. Your code for last two cond branches:

((atom (car form))
         (print (car form))
         (cons (car form) (list lst)) ;; Result is ignored
         (delete-duplicates lst)      ;; Result is ignored
         (constants-aux (cdr form) lst))

You should write:

(constant-aux (cdr form) (delete-duplicates lst))

        (T
           (constants-aux (car form) lst) ;; Result is ignored
           (constants-aux (cdr form) lst))))

May be (depends on what you want to get) you should write:

(cons
   (constants-aux (car form) lst)
   (constants-aux (cdr form) lst))

4 I'm not shure, but it looks like you using print for debug, just use trace instead. For your code it'll give you pretty good information what is going on with your list during execution:

  0: (CONSTANTS-AUX (IMPLIES (NOT Q) (IMPLIES Q P)) NIL)
    1: (CONSTANTS-AUX ((NOT Q) (IMPLIES Q P)) NIL)
      2: (CONSTANTS-AUX (NOT Q) NIL)
        3: (CONSTANTS-AUX (Q) NIL)

Q           4: (CONSTANTS-AUX NIL NIL)
          4: CONSTANTS-AUX returned NIL
        3: CONSTANTS-AUX returned NIL
      2: CONSTANTS-AUX returned NIL
      2: (CONSTANTS-AUX ((IMPLIES Q P)) NIL)
        3: (CONSTANTS-AUX (IMPLIES Q P) NIL)
          4: (CONSTANTS-AUX (Q P) NIL)

Q             5: (CONSTANTS-AUX (P) NIL)

P               6: (CONSTANTS-AUX NIL NIL)
              6: CONSTANTS-AUX returned NIL
            5: CONSTANTS-AUX returned NIL
          4: CONSTANTS-AUX returned NIL
        3: CONSTANTS-AUX returned NIL
        3: (CONSTANTS-AUX NIL NIL)
        3: CONSTANTS-AUX returned NIL
      2: CONSTANTS-AUX returned NIL
    1: CONSTANTS-AUX returned NIL
  0: CONSTANTS-AUX returned NIL

5. It'll be really easyer to answer if you explain what transformation you expecting from this code.

Good luck.

Upvotes: 8

Related Questions