Re: my first Lisp code...comments welcome

ben.caxton@xxxxxxxxx writes:

> ((= k 0)
> (if (= n 0) (list ()) nil))

Don't use the notation () as an unadorned expression in code.
It can be used in text, to denote an empty list that is part of a form.
For example:
(defun foo () 'foo)
In that case, () is not itself a form but is a sub-expression of a
DEFUN form, so it's ok.
If you mean an empty list, use a quote mark in front of it. e.g.,
(defun the-empty-list () '())
If you mean false, use nil (as if it were a variable holding false,
ignoring that nil is also the false value itself).
If you mean nil in the role of a symbol, use 'nil to highlight that
you are not thinking of its value (even though its value is also nil).

It's sometimes hard for newbies to understand when to use '() and when
to use nil. e.g., which does MEMBER return on a failing search. The
answer in that case is that on a succeeding search, MEMBER returns the
list tail whose car is the sought item. Since that's not true in the failing
case, what is returned is not a list tail and is instead just false. So
in that case, return NIL.

In your case here, if I read your code right, you're returning something
that on a recursion will be always appended, you want to say
(if (= n 0) (list '()) '())
This will help readers of your code know the intended view of your
nils here are both as empty lists, not as falses nor as symbols. It will
also keep you from confusing program text with program data.

> ;; obviously one composition of something into one part: return a
> ;; list containing that one composition
> ((= k 1)
> (list (list n)))
> ;; otherwise, recurse based on first element of the composition
> (t
> (mapcar (lambda (i)
> (mapcar (lambda (comp)
> (cons i comp)) (compositions (- n i) (- k
> 1))))
> (cons 0 (range n))))))

Except in very rare special cases, and function calls are not one of them,
don't ever start subform n of an expression on the same line as subform n-1
if you have already introduced a newline between subforms for previous
subforms of the expression. Never
(f
a b)
always either
(f a b)
or
(f a
b)
or
(f
a
b)
An example of an exception might be setq, where there is a certain structure
to the apparent linear nature of the args, so
(setq a b
c d)
is ok as a kind of special case. But you want
(mapcar (lambda (i)
(mapcar (lambda (comp) (cons i comp))
(compositions (- n i) (- k 1))))
(cons 0 (range n)))
above. This helps you more easily tell how many subforms something has
just by its indentation.

And don't indent by 4, indent by 2.

> It uses this function:
>
> ;;; from http://www.cs.berkeley.edu/~fateman/mma1.6/comb.lisp
> (defun range (n &aux ans)
> "Return a list of elements from 1 to n"
> (declare (fixnum n))
> (do ((i n (1- i)))
> ((= i 0) ans)
> (declare (fixnum i))
> (setq ans (cons i ans))))

In macros with a body, let the header forms be indented as you've done
but move the body back to an indentation level of 2 so you can see
where the header is done

(do ((i n (1- i)))
((= i 0) ans)
(declare (fixnum i))
(setq ans (cons i ans)))

Use PUSH instead of cons in the last part.
(push i ans) instead of (setq ans (cons i ans))
It makes it easier to see the idiom you're using.

Don't use &aux except in rare cases, usually internal to macros.
It's stylistically pretty yucky even though it can get you out of certain
odd and extremely obscure problems in macros. e.g.,

(defun range (n)
"Return a list of elements from 1 to n"
(declare (fixnum n))
(do ((ans '() (cons i ans))
(i n (1- i)))
((= i 0) ans)
(declare (fixnum i))))

Learn about DOTIMES, which is often easier to read than DO.

(defun range (n)
(let ((result '()))
(dotimes (i n) (push (1+ i) n))
(nreverse result)))

This is controversial with some, but over my career I've come to believe
it more and more: Learn about LOOP, which embraces a lot of idioms in
very much more readable ways than DO and, when used right, is still
easier to read than either DO or DOTIMES.

(defun range (n)
(loop for i from 1 to n
collect i))

Spell out variables. "answer" or "result", not "ans".
"ans" makes me wonder if it's a plural of an "an, whatever an "an" is.

Are you sure really need to make RANGE return only fixnums? The function
would be more general returning also bignums. You shouldn't have to write
a separate BIGNUM-RANGE if you end up needing one. Think reusable.
When you get to the point of delivering a commercial product and it runs
too slowly, clean it up by adding declarations. But don't prematurely
optimize your code now thinking you are making it better. I think you're
just slowing your coding, making it take more time and risking early success,
making it more fragile to needed evolution in the early stages. But that's
just me.

Hope that helps.
--Kent
.

Relevant Pages

  • Re: The empty list and the end of a list
    ... takes a list (tree) as input and returns a list of all atoms. ... The embedded NIL differs from the ending NIL in its syntactic ... (atom (list tree)) ... NIL atoms, rather than flattening them as if they denoted empty lists, ...
    (comp.lang.lisp)
  • Re: The empty list and the end of a list
    ... takes a list (tree) as input and returns a list of all atoms. ... The embedded NIL differs from the ending NIL in its syntactic ... NIL atoms, rather than flattening them as if they denoted empty lists, ... (atom (list tree)) ...
    (comp.lang.lisp)
  • Re: NIL is not of type CONS
    ... In Scheme, the empty list is ', not NIL. ... Many people around here know Scheme very well. ... The Scheme representation of lists and booleans introduces numerous ... a WIDGET class, you can pretend that there is a WIDGET* class which is ...
    (comp.lang.lisp)
  • Re: NIL is not of type CONS
    ... In lisp, 'means NIL. ... Common Lisp actually has a concept of uninitialized (unbound) variables, ... It's also the case that may not help you much, because you cannot distinguish between lists and booleans here. ...
    (comp.lang.lisp)
  • Re: how to speed up some lisp code?
    ... >> representing these as lists. ... declare the variables to be FIXNUM. ...
    (comp.lang.lisp)