lisp program error

Discussion of Common Lisp
Post Reply
haplo
Posts: 4
Joined: Sun Oct 26, 2008 2:01 pm

lisp program error

Post by haplo » Sun Oct 26, 2008 2:16 pm

Hello! I'm currently working on a lisp program that should increment a number stored in a list through its digits by 1. I've written a module that recieves the reversed list and a carry flag but it won't even compile, so I can't tell if I'm on the right path or not. Can you please look at the code and tell me where I'm screwing up? I'm a complete noob so it's probably just a syntax error. Thank you very much in advance.

Code: Select all

(DEFUN incr (X N)
	(COND
		((NULL X) 
			(COND
				((= N 1) (1))
				(T ())
			)
		)
		(T 
			(+ (CAR X) 1)
			(COND
				((> (CAR X) 9) (- (CAR X) 10) (CONS ((CAR X) (incr (CDR X) 1))))
				(T (CONS (CAR X) (incr (CDR X) 0)))
			)
		)
	)
)
Oh, and the error I get is:

Code: Select all

;; Loading file /home/zephyr/Desktop/PLF/incr.lsp ...
WARNING: DEFUN/DEFMACRO: redefining function INCR in
         /home/zephyr/Desktop/PLF/incr.lsp, was defined in
         /home/zephyr/Desktop/PLF/incr2.lsp
*** - SYSTEM::%EXPAND-FORM: invalid form (1)
The following restarts are available:
SKIP           :R1      skip (DEFUN INCR # ...)
STOP           :R2      stop loading file /home/zephyr/Desktop/PLF/incr.lsp
ABORT          :R3      ABORT
ABORT          :R4      ABORT

Exolon
Posts: 49
Joined: Sat Jun 28, 2008 12:53 pm
Location: Ireland
Contact:

Re: lisp program error

Post by Exolon » Sun Oct 26, 2008 3:47 pm

Hi, in your code you have:

Code: Select all

(CONS ((CAR X) (incr (CDR X) 1)))
CONS takes two arguments, but you've wrapped them up as one (a common mistake when coming from a function(arg1, arg2)-style language, and one I make as well):

Code: Select all

(CONS (CAR X) (incr (CDR X) 1))
With that removed I can compile and call the function, but I haven't noticed any effects:

Code: Select all

CL-USER> (incr (list 1 2 3) 0)
(1 2 3)
CL-USER> (incr (list 1 2 3) 1)
(1 2 3)
CL-USER> (incr (list 1 2 3) 10)
(1 2 3)
CL-USER> (incr (list 1 2 3 4) 10)
(1 2 3 4)

qbg
Posts: 64
Joined: Mon Jun 30, 2008 1:05 pm
Location: Minnesota

Re: lisp program error

Post by qbg » Sun Oct 26, 2008 10:08 pm

First, about the way you are formatting your code:
1) Why are you using uppercase for the symbols?
2) Those closing parens feel lonely on their own line

Now, what are you trying to do in incr? Are you wanting a function that is something like:

Code: Select all

(defun incr (list n)
  "Increment each element in the list by n"
  (mapcar (lambda (x) (+ x n)) list))
?

Paul Donnelly
Posts: 148
Joined: Wed Jul 30, 2008 11:26 pm

Re: lisp program error

Post by Paul Donnelly » Sun Oct 26, 2008 11:40 pm

Your "illegal function call" is because you typed "(1)". You can't call 1. Probably you wanted (list 1).

Your second issue (aside from the improper call to CONS, mentioned already) is that in your CONDs you're doing some addition and doing nothing with the return values.

Code: Select all

(defun incr (x n)
  (cond ((null x) (cond ((= n 1) (list 1))
                        (t ())))
        (t (+ (car x) 1) ; <--- Here
           (cond ((> (car x) 9)
                  (- (car x) 10) <--- Here
                  (cons (car x) (incr (cdr x) 1)))
                 (t (cons (car x) (incr (cdr x) 0)))))))
Where you use COND, it would be more normal to use IF, to avoid extra parens, the explicit (t ... ) clause, and to make it clear that there are only two branches.

Code: Select all

(defun incr2 (x n)
  (if (null x)
      (if (= n 1) (list 1)
          '())
      (progn (+ (car x) 1) ; This addition still does nothing.
             (if (> (car x) 9)
                 (progn (- (car x) 10) ; This subtraction still does nothing.
                        (cons (car x) (incr (cdr x) 1)))
                 (cons (car x) (incr (cdr x) 0))))))
I'm not really sure what you're trying to do, but was it something like this?

Code: Select all

(defun incr3 (x) ; I have no idea what N was for.
  (mapcar (lambda (y) (mod (1+ y) 10))
          x))
Mapcar will call a provided function on each element of a list, and return a new list containing the result of that function for each item in the original list. You can look it up in the Hyperspec for a more complete description.

If you wanted a recursive version, it is here. Don't peek if you wanted to do it yourself. Keep in mind that you don't have to reimplement MOD, and you especially don't need to do it inline—even if you did have to write it yourself, you should have put it in another function (named MOD, perhaps). Also keep in mind that addition does not modify anything. It just returns a value. This recursive function is not tail recursive, so it is sure to fail on long lists. A tail recursive version follows. If tail calls are optimized in your Lisp, this one will work on arbitrarily long lists.

Code: Select all

(defun incr5 (x &optional a)
  (if (null x)
      (nreverse a)
      (incr5 (cdr x) (cons (mod (1+ (car x)) 10) a))))
My use of NULL is technically superfluous.

Code: Select all

(defun incr6 (x &optional a)
  (if x
      (incr5 (cdr x) (cons (mod (1+ (car x)) 10) a))
      (nreverse a)))

Exolon
Posts: 49
Joined: Sat Jun 28, 2008 12:53 pm
Location: Ireland
Contact:

Re: lisp program error

Post by Exolon » Mon Oct 27, 2008 3:03 am

Paul Donnelly wrote:(if x ...
Ahh, we can do that? I wanted that a couple of days ago and ended up writing "(if (not (null ...)))", which smells. Funny, I used to use "if(x)" in C/C++ quite often.

Paul Donnelly
Posts: 148
Joined: Wed Jul 30, 2008 11:26 pm

Re: lisp program error

Post by Paul Donnelly » Mon Oct 27, 2008 7:42 am

Exolon wrote:
Paul Donnelly wrote:(if x ...
Ahh, we can do that? I wanted that a couple of days ago and ended up writing "(if (not (null ...)))", which smells. Funny, I used to use "if(x)" in C/C++ quite often.
Yep. In Common Lisp, nil (which is the same as '(), the empty list) is false, and everything else is true, including zero.

haplo
Posts: 4
Joined: Sun Oct 26, 2008 2:01 pm

Re: lisp program error

Post by haplo » Mon Oct 27, 2008 8:42 am

Wow, you guys are great! It worked, but only after I made all modifications you specified. I really didn't expect answers to come in this soon (i didn't expect one at all actually).
@Exolon: Thanx man, you saved my life, I would've never have spotted such a dumb mistake. Now that I got that to work I managed to check its result and do the necessary modifications in order to have it do what it should.
@qbg: I'm using uppercase since that's what the examples I've read use.I know it's not really necessary but it adds a bit of readability, since I don't have a good IDE for writing code in LISP. The parantheses are aligned like that to add some more ease in reading the code by trying to order it in blocks. The program should take a list with a number stored by its digits and increase the number by one. The function that does this has to be recursive.
@Paul Donnelly: Thank you for drawing my attention to that (using (1) instead of '(1) ), I don't use ifs because the program is for a course of logical and functional programming and it would be awkward if I started using new keywords to complete my laboratories. I want to keep things simple and straight to the point when I do the laboratory tasks. But i'll research ifs as you suggested since they'll surely help me with the written or practical exams.
That being said here's the final code, I couldn't have done this without your help.

Code: Select all

(DEFUN inc (X)
    (REVERSE (incr (REVERSE X) 1))
)

(DEFUN incr (X N)
	(COND
		((NULL X) 
			(COND
				((= N 1) '(1))
				(T NIL)
			)
		)
		(T 
			(COND
			        ((= N 0) 
				    (CONS (CAR X) (incr (CDR X) 0))
				)
				(T
				    (COND
				        ((= (CAR X) 9) (CONS (- (CAR X) 9) (incr (CDR X) 1)))
				        (T (CONS (+ (CAR X) 1) (incr (CDR X) 0)))
				    )
				)
			)
		)
	)
)
Oh, and if you have any suggestions concerning solving this problem in a more orthodox way please tell me your opinions as I have first solved it in c recursively and translated the code in lisp.

qbg
Posts: 64
Joined: Mon Jun 30, 2008 1:05 pm
Location: Minnesota

Re: lisp program error

Post by qbg » Mon Oct 27, 2008 10:43 am

haplo wrote: @qbg: I'm using uppercase since that's what the examples I've read use.I know it's not really necessary but it adds a bit of readability, since I don't have a good IDE for writing code in LISP. The parantheses are aligned like that to add some more ease in reading the code by trying to order it in blocks. The program should take a list with a number stored by its digits and increase the number by one. The function that does this has to be recursive.
Emacs will help with formatting code the standard way. (There exists a windows version if you are using that) LispWorks Personal Edition's built in editor is good and it is a free download (though not free software). For reading code in blocks, the standard style shows blocks by indentation.

Now then, are you wanting incr to do this:
(incr '(7 8 9)) => (7 9 0)
?
Oh, and if you have any suggestions concerning solving this problem in a more orthodox way please tell me your opinions as I have first solved it in c recursively and translated the code in lisp.
If my interpretation above is correct, here is a somewhat non-orthodox way to do it:

Code: Select all

(defun incr (x)
  (let ((carry 1) (list (reverse x)))
    (labels ((incr-aux (rest-x)
               (incf (car rest-x) carry)
               (if (> (car rest-x) 9)
                   (setf carry 1 (car rest-x) (- (car rest-x) 10))
                 (setf carry 0))
               (if (cdr rest-x)
                   (incr-aux (cdr rest-x)))))
      (incr-aux list)
      (nreverse list))))
Depending on how of a beginner you are, you may want to think of why it works.

Paul Donnelly
Posts: 148
Joined: Wed Jul 30, 2008 11:26 pm

Re: lisp program error

Post by Paul Donnelly » Mon Oct 27, 2008 12:53 pm

haplo wrote:The program should take a list with a number stored by its digits and increase the number by one.
Oh, so it's like

Code: Select all

(incr '(1 2 9)) ==> (1 3 0)
I missed something I should have pointed out earlier. COND is a like a chain of if/then/else if/then/else if/then/etc. You can eliminate all those "(t (cond ...".

Code: Select all

(defun incr2 (x n)
  (cond
    ((null x)
     (cond
       ((= n 1) '(1))
       (t nil)))
    ((= n 0)
     (cons (car x) (incr (cdr x) 0)))
    ((= (car x) 9) (cons (- (car x) 9) (incr (cdr x) 1)))
    (t (cons (+ (car x) 1) (incr (cdr x) 0)))))

Post Reply