tags:

views:

318

answers:

4

I have some code which collects points (consed integers) from a loop which looks something like this:

(loop
    for x from 1   to     100
    for y from 100 downto 1
        collect `(,x . ,y))

My question is, is it correct to use `(,x . ,y) in this situation?

Edit: This sample is not about generating a table of 100x100 items, the code here just illustrate the use of two loop variables and the consing of their values. I have edited the loop to make this clear. The actual loop I use depends on several other functions (and is part of one itself) so it made more sense to replace the calls with literal integers and to pull the loop out of the function.

+1  A: 

Why not just

(cons x y)

By the way, I tried to run your code in CLISP and it didn't work as expected. Since I'm not a big fan of the loop macro here's how you might accomplish the same thing recursively:

(defun genint (stop)
  (if (= stop 1) '(1)
      (append (genint (- stop 1)) (list stop))))

(defun genpairs (x y)
  (let ((row (mapcar #'(lambda (y)
                        (cons x y))
                        (genint y))))
    (if (= x 0) row
        (append (genpairs (- x 1) y)
                row))))

(genpairs 100 100)
Kyle Cronin
Beat me by 9 seconds, oh why did I have to elaborate? ;o)
leppie
The code was just an example, I have 'cleverer' code determining what the loop is looking for, not making a table.
dsm
Oh OK, I didn't know if you were asking because your code was broken.
Kyle Cronin
+7  A: 

It would be much 'better' to just do (cons x y).

But to answer the question, there is nothing wrong with doing that :) (except making it a tad slower).

leppie
+5  A: 

I think the answer here is resource utilization (following from This post)

for example in clisp:

[1]> (time
         (progn
             (loop
                 for x from 1 to 100000
                 for y from 1 to 100000 do
                     collect (cons x y))
         ()))
WARNING: LOOP: missing forms after DO: permitted by CLtL2, forbidden by ANSI
         CL.
Real time: 0.469 sec.
Run time: 0.468 sec.
Space: 1609084 Bytes
GC: 1, GC time: 0.015 sec.
NIL
[2]> (time
         (progn
             (loop
                 for x from 1 to 100000
                 for y from 1 to 100000 do
                     collect `(,x . ,y)) ;`
         ()))
WARNING: LOOP: missing forms after DO: permitted by CLtL2, forbidden by ANSI
         CL.
Real time: 0.969 sec.
Run time: 0.969 sec.
Space: 10409084 Bytes
GC: 15, GC time: 0.172 sec.
NIL
[3]>
dsm
+3  A: 

dsm: there are a couple of odd things about your code here. Note that

(loop for x from 1 to 100000
  for y from 1 to 100000 do
  collect `(,x . ,y))

is equivalent to:

(loop for x from 1 to 100
   collecting (cons x x))

which probably isn't quite what you intended. Note three things: First, the way you've written it, x and y have the same role. You probably meant to nest loops. Second, your do after the y is incorrect, as there is not lisp form following it. Thirdly, you're right that you could use the backtick approach here but it makes your code harder to read and not idiomatic for no gain, so best avoided.

Guessing at what you actually intended, you might do something like this (using loop):

(loop for x from 1 to 100 appending 
  (loop for y from 1 to 100 collecting (cons x y)))

If you don't like the loop macro (like Kyle), you can use another iteration construct like

(let ((list nil)) 
   (dotimes (n 100) ;; 0 based count, you will have to add 1 to get 1 .. 100
     (dotimes (m 100) 
       (push (cons n m) list)))
   (nreverse list))

If you find yourself doing this sort of thing a lot, you should probably write a more general function for crossing lists, then pass it these lists of integers

If you really have a problem with iteration, not just loop, you can do this sort of thing recursively (but note, this isn't scheme, your implementation may not guaranteed TCO). The function "genint" shown by Kyle here is a variant of a common (but not standard) function iota. However, appending to the list is a bad idea. An equivalent implementation like this:

(defun iota (n &optional (start 0))
  (let ((end (+ n start)))
    (labels ((next (n)
               (when (< n end) 
                 (cons n (next (1+ n))))))
      (next start))))

should be much more efficient, but still is not a tail call. Note I've set this up for the more usual 0-based, but given you an optional parameter to start at 1 or any other integer. Of course the above can be written something like:

(defun iota (n &optional (start 0))
  (loop repeat n 
     for i from start collecting i))

Which has the advantage of not blowing the stack for large arguments. If your implementation supports tail call elimination, you can also avoid the recursion running out of place by doing something like this:

(defun iota (n &optional (start 0))
  (labels ((next (i list)
             (if (>= i (+ n start))
                 nil
                 (next (1+ i) (cons i list)))))
    (next start nil)))

Hope that helps!

simon
You're right, there are more efficient ways of accomplishing what I was doing. I was trying to duplicate what I thought was the intended output of the script as quickly as possible. I'm not a CL user (I'm a Schemer) so I didn't know if CL had any number list creating functions, so thanks for the tip
Kyle Cronin
This is a great answer!
jfm3
This is indeed a great answer, but as I noted, the broken loop was just an illustration. I have now edited the question to better reflect the intention of the loop (consing variables, rather than generating a table). Thanks for your answer.
dsm
Ok, I commented above about your original intent. Everything in this answer generalizes though, so it's not a loss...
simon