0
\$\begingroup\$

Code below is answer to an exercise in Paul Graham's ANSI Common Lisp (ex. 4.1). The challenge is to rotate an array by 90 degrees. I found an example solution which I've commented below.

It seems ok'ish and I'm enjoying learning how to use do, but in this particular case it seems actually more verbose than the familir c-style nested for loops to process a 2d array. I'm wondering if the below is good style or if it can be improved upon?

Is the answer going to be the loop macro? I'm avoiding that at the moment while learning, waiting till I have better mastery of the basics. Yet would like to not pick up bad habits. Hence this question about style regarding the below.

(defun quarter-turn (arr)
  (let* ((dim (array-dimensions arr))  ; let* sets sequentially, not in parallel.
         (row (first dim))
         (col (second dim))
         (n row)  ; initialise n to row, just to organise
         new-arr) ; shorthand for (new-arr nil)
    (cond ((not (= row col))
           (format t "The arg is not a square array.~%"))
          (t (setf new-arr (make-array dim :initial-element nil))
             (do ((i 0 (+ i 1)))  ; Q. surprisingly, lisp's 'do' here looks more
                 ((= i n))        ; verbose than c-style 'for'. Is this good style?
                 (do ((j 0 (+ j 1)))
                     ((= j n))
                     (setf (aref new-arr j i) (aref arr (- n i 1) j))))
             new-arr))))

Source for the above code: http://www.cs.uml.edu/~lhao/ai/lisp/ansi-common-lisp/solution-ch04.htm

\$\endgroup\$

closed as off-topic by 200_success, pacmaninbw, Snowbody, Mast, Grajdeanu Alex. Apr 21 at 15:11

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Authorship of code: Since Code Review is a community where programmers improve their skills through peer review, we require that the code be posted by an author or maintainer of the code, that the code be embedded directly, and that the poster know why the code is written the way it is." – 200_success, pacmaninbw, Snowbody, Mast, Grajdeanu Alex.
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ You didn't write this yourself, did you? Please take a look at the help center. \$\endgroup\$ – Mast Apr 20 at 11:45
  • \$\begingroup\$ I have taken a look at the help center. The code is a solution to an exercise from a well known lisp book, which I tried to solve myself, but had no idea how to. So, I searched online and found a solution which seemed to do what was asked. I typed it in and ran the code. But I was not entirely happy with the solution and had questions I wanted to ask about it. The comments in the code are my own, forming the questions I wanted to discuss about the code. I also attributed the source as seen above. I understand the rules now, but had not realised I was falling foul of them when making this post. \$\endgroup\$ – mwal Apr 22 at 15:39
1
\$\begingroup\$

A few comments about your code:

  1. Why introduce a new variable n copy of another variable? If you want to be more concise you could use n and m insted of row and col (that I don't like because they are singular names, while they represent the number of rows and of columns).
  2. Why define new-arr before the test and then assign it after the test of squareness? Don't by shy of use let as many times as you need, to introduce variables when they are really necessary.
  3. Use assert instead of cond to check for correct parameters: in this way you give the user the opportunity of correct them, and produce a shorter and more readable code.
  4. Why initialize the new array to a value nil which is immediately overwritten?
  5. The fist do could return the result, so to avoid the last line of the function.
  6. Use the primitive function 1+ instead of summing by 1.
  7. Document the function with a comment about its scope as first form of the body of the function.

So, after this first set of comments, the function could be rewritten as:

(defun quarter-turn (arr)
  "rotate a square matrix 90° clockwise"
  (let* ((dim (array-dimensions arr))
         (n (first dim))
         (m (second dim)))
    (assert (= n m) (arr) "The argument is not a square array.")
    (let ((new-arr (make-array dim)))
      (do ((i 0 (1+ i)))
          ((= i n) new-arr)
        (do ((j 0 (1+ j)))
            ((= j n))
          (setf (aref new-arr j i) (aref arr (- n i 1) j)))))))

DO versus LOOP

As you have already noted, in this case do is more verbose than loop. I think that in a case like this they are more or less equivalent (just a personal opinion). I would prefer the loop form not only because it seems to me slightly more concise, but also because it is more “natural” for me to think of a classical nested loop for a bidimensional array in terms two nested loop forms:

(defun quarter-turn (arr)
  "rotate a square matrix 90° clockwise"
  (let* ((dim (array-dimensions arr))
         (n (first dim))
         (m (second dim)))
    (assert (= n m) (arr) "The argument is not a square array.")
    (let ((new-arr (make-array dim)))
      (loop for i below n
            do (loop for j below n
                     do (setf (aref new-arr j i) (aref arr (- n i 1) j))))
      new-arr)))
\$\endgroup\$
  • \$\begingroup\$ Thanks @Renzo that's a beautiful answer. \$\endgroup\$ – mwal Apr 22 at 15:52
  • \$\begingroup\$ I noticed that, in your solutions, let replaces a setf compared to the one I posted. That seems strikingly interesting. setf seems one kind of scope, let another. I'm not quite sure how to pose this question correctly, but can/does let replace setf? \$\endgroup\$ – mwal Apr 23 at 7:53
  • \$\begingroup\$ As a thought experiment, I was trying to establish whether the second setf could be replaced somehow with let. Tentatively it appears perhaps not. I believe there might be a name for this :) \$\endgroup\$ – mwal Apr 23 at 8:07
  • \$\begingroup\$ Trying not to get confused here... I thought I'd encountered 'lexical scope' but on a quick check of what that could mean it appears i'm not quite ready for that yet. So, given the current question, would it be correct to say "let creates a new lexical context, so if we tried to use it to replace the second setf above, it could reference, but it couldn't set, new-arr". ? \$\endgroup\$ – mwal Apr 23 at 8:31
  • 1
    \$\begingroup\$ So, setf on a variable requires that the variable already exists, while with let the variable is created (even if a variabile with the name introduced already exists, it is “hidden” by the new definition, which holds only for the body of let). \$\endgroup\$ – Renzo Apr 23 at 9:24

Not the answer you're looking for? Browse other questions tagged or ask your own question.