Don't take all remarks as *absolute*. Some are just how *I* would do things. There are many ways to structure a program. It's a great way to learn Lisp programming! **Good:** - documented constant values with speaking names - names are usually speaking and self-documenting - use of CLOS - use of documentation strings **Suggestions for improvements:** - DEFCLASS has a :documentation option, could be used for documentation - DEFCLASS slots have type options, could be used to declare&document the legal types of the slot values - class PLAYER has accessors, class GAME not? - in class GAME the slot CURRENT-PLAYER is a number, but sounds like it would be a PLAYER object - in class GAME, what is the slot NUM-PLAYERS for? - *provides operations for playing GREED.* In CLOS classes don't really provide operations. - the purpose of the macro `DEFGAME` is unclear to me. I would rather write a function MAKE-GAME or CREATE-GAME. This function then would check the number of players (coming in as an argument) with something like CHECK-TYPE or ASSERT. Since you don't handle the CONDITION, a simple error would be sufficient: `(check-type number-of-players integer)` or `(assert (integerp number-of-players) (number-of-players) "The number of players needs to be an integer")`. - MAKE-COUNTS: you can use INCF directly with one GETHASH form. Example: (defun make-counts (dice &aux (nums-hash (make-hash-table))) (dolist (num dice nums-hash) (incf (gethash num nums-hash 0)))) - MAKE-PLAYERS: the element-type argument is typically not useful. This argument in Common Lisp is only used in Common Lisp to allocate specialized arrays, but for player there will be no specialized array. All you will get is the usual array, which has an element type of `T`. - MAKE-PLAYER: you can also make a list of players and coerce the list into a VECTOR - GET-HIGH-SCORER: you can for example sort a copy of the list and get highest scorer(s) - DISPLAY-SCORE: you could use `(LOOP FOR player ACROSS (players game) do ...) Generally: - would make a difference between an object and an index to an object. CURRENT-PLAYER vs. CURRENT-PLAYER-NUMBER - I would structure the file not around constants, classes, methods for class 1, ... methods for class N. I would structure my file around the domain: Example: setup: package, helper stuff managing domain class 1: game managing domain class 2: player domain functionality 1: setup of the game domain functionality 2: start of a game domain functionality 3: playing a turn domain functionality 4: scoring domain functionality 5: end of a game