Dom De Felice <dom.fel...@gmail.com> writes:
> Being this is my first working program in Common Lisp I would be very
> glad to receive comments/opinions/suggestions about the source code
> and what should I improve in my CL coding style.
Hi Dom, some things I've noticed, that is imho:
- the return value of DELETE(-IF) has to be used, it doesn't necessarily
modify the given sequence, or it might remove the first element of the
sequence,
- GAME-STEP calls itself and PLAY recursively, which isn't guaranteed to
be optimized away by a tail-call optimization, so do an explicit loop
in PLAY or on another outer layer,
and more stylistic things:
- add an option to quit the game (user input 0?) (and perhaps check
for wrong input too ...),
- multiple SETF clauses can be written in one statement, e.g.
> (setf (player-scopas human) 0
> (player-scopas pc) 0)
- "with i = 1 do ... (incf i)" is the same as "for i from 1 do"
- PRINC and TERPRI together probably aren't as clear as a single FORMAT
call; in any case, a single FORMAT call is easier to edit later on,
- you could use PRINT-OBJECT instead of implementing separate DESCRIBE-
functions, e.g. specialize it on the structs GAME, PLAYER, CARD, then
simply print the object on the output stream to describe them,
- since you already use formatstrings quite often, you could also
rewrite DESCRIBE-OPTION using a conditional as:
> (defun describe-option (option)
> (let ((played-card (car option))
> (collected-cards (cdr option)))
> (format t "Play ~A~@[ taking ~{~A~^, ~}~]~%"
> (card-description played-card)
> (mapcar #'card-description collected-cards))))
- move the comments into docstrings, e.g. for POSSIBLE-OPTIONS or
-PLAYING the comment already describes the function very detailed,
- is there another reason to use a list in HAND-SCORES-ASSIGMENT other
than the COUNT calls in UPDATE-SCORES? I'd use a structure here too.
- FROM-DECK and INTO-DECK could be called with unquoted symbols
(like (from-deck hand ...)) if you move the quote to the macro
definition, e.g. write them as
> (defmacro from-deck (deck &body body)
> `(let ((*from-deck* (deck ',deck)))
> ,@body))
Hopefully I've made no errors ... anyway, regards and hth,
Olof