I'm really new to common Lisp and having some struggles. I'm working on a function that given x, y and an array with the index for vertical value returns NIL if there's any element diagonal from (x y).
(defun diagonal? (x y array)
(loop for line from 0 to 19 do
(let (col (aref array line)) (
(if (= col -1) (return-from diagonal? t))
(let (diag (= (abs (- x line)) (abs (- y col)))) (
if (= diag T) (return-from diagonal? NIL))
)
)))
return T
)
When I try this function however, I get the following error:
; caught ERROR:
; The LET binding spec (AREF ARRAY LINE) is malformed.
; (SB-INT:NAMED-LAMBDA DIAGONAL?
; (X Y ARRAY)
; (BLOCK DIAGONAL?
; (LOOP FOR LINE FROM 0 TO 19
; DO (LET (COL #)
; (# #)))
; RETURN
; T))
First and extremely important: use automatic indentation.
(defun diagonal? (x y array)
(loop for line from 0 to 19 do
(let (col (aref array line)) (
(if (= col -1) (return-from diagonal? t))
(let (diag (= (abs (- x line)) (abs (- y col)))) (
if (= diag T) (return-from diagonal? NIL))
)
)))
return T
)
Then your code looks strange with long lines: never put parentheses on their own line and never end a line with an open parenthesis.
Improved:
(defun diagonal? (x y array)
(loop for line from 0 to 19 do
(let (col (aref array line))
((if (= col -1)
(return-from diagonal? t))
(let (diag (= (abs (- x line))
(abs (- y col))))
(if (= diag T)
(return-from diagonal? NIL))))))
return T)
Second: LET
expects a list of bindings. A single binding is a variable or (variable value)
:
(defun diagonal? (x y array)
(loop for line from 0 to 19 do
(let ((col (aref array line)))
((if (= col -1)
(return-from diagonal? t))
(let ((diag (= (abs (- x line))
(abs (- y col)))))
(if (= diag T)
(return-from diagonal? NIL))))))
return T)
Third: LET expects a body of Lisp forms. That is zero or more Lisp forms:
(defun diagonal? (x y array)
(loop for line from 0 to 19 do
(let ((col (aref array line)))
(if (= col -1)
(return-from diagonal? t))
(let ((diag (= (abs (- x line))
(abs (- y col)))))
(if (= diag T)
(return-from diagonal? NIL)))))
return T)
Fourth: =
expects numbers as arguments. T
is not a number. =
already returns T
or NIL
which we can test.
(defun diagonal? (x y array)
(loop for line from 0 to 19 do
(let ((col (aref array line)))
(if (= col -1)
(return-from diagonal? t))
(if (= (abs (- x line))
(abs (- y col)))
(return-from diagonal? NIL))))
return T)
Fifth: return T
is not a valid Lisp form. We can just return T
directly.
(defun diagonal? (x y array)
(loop for line from 0 to 19 do
(let ((col (aref array line)))
(if (= col -1)
(return-from diagonal? t))
(if (= (abs (- x line))
(abs (- y col)))
(return-from diagonal? NIL))))
T)
Sixth: we don't need the LET
for col
, we can replace it with another FOR
in the LOOP
.
(defun diagonal? (x y array)
(loop for line from 0 to 19
for col = (aref array line)
do
(if (= col -1)
(return-from diagonal? t))
(if (= (abs (- x line))
(abs (- y col)))
(return-from diagonal? NIL))))
T)
Seventh: multiple IF
can be written as a single COND
.
(defun diagonal? (x y array)
(loop for line from 0 to 19
for col = (aref array line)
do (cond ((= col -1)
(return-from diagonal? t))
((= (abs (- x line))
(abs (- y col)))
(return-from diagonal? nil))))
t)
Eigth: for from 0 to n
can be replaced by below (+ n 1)
or upto n
(defun diagonal? (x y array)
(loop for line below 20
for col = (aref array line)
do (cond ((= col -1)
(return-from diagonal? t))
((= (abs (- x line))
(abs (- y col)))
(return-from diagonal? nil))))
t)
Ninth: since (RETURN-FROM ... T)
returns from a function which returns T
explicitly by default, we can replace it with an UNTIL
clause in the loop:
(defun diagonal? (x y array)
(loop for line below 20
for col = (aref array line)
until (= col -1)
when (= (abs (- x line))
(abs (- y col)))
do (return-from diagonal? nil))
t)
Tenth: since col is just the iterating the values of the array:
(defun diagonal? (x y array)
(loop for line below 20
for col across array
until (= col -1)
when (= (abs (- x line))
(abs (- y col)))
do (return-from diagonal? nil))
t)
Eleventh: suggestion by @Coredump, use NEVER
. The default return value of the LOOP
is now T
. Return only then nil
, when the never
clause fails.
(defun diagonal? (x y array)
(loop for line below 20
for col across array
until (= col -1)
never (= (abs (- x line))
(abs (- y col)))))