From: w8
Subject: My First Lisp Program
Date: 
Message-ID: <1188998214.855657.326570@57g2000hsv.googlegroups.com>
Hi,

I'm a novice to Common Lisp, just finished reading the first half of
Practical Common Lisp and managed to hack out a tic-tac-toe program.
I'm just wondering whether the style I am writing is that of Lisp. I
would be grateful if any one would take a look at it and give me
suggestion for improvement. Thanks a lot in advance.

;;;; Created on 2007-08-27 20:27:05
;;;; A tic-tac-toe game

(defparameter player-names (cons nil nil))

;;;------------- condition section
(define-condition invalid-position (error)
  ((text :initarg :text :reader text)))

(define-condition end-of-game (error)
  ((text :initarg :text :reader text)))

;;;------------- general utilities section
----------------------------

;;utilities
(defun filter (fn lst) ;not used?
  "Filter lst by a function fn."
  (let ((acc nil))
    (dolist (x lst)
      (let ((val (funcall fn x)))
        (if val (push val acc))))
    (reverse acc)))

(defun random-list-element (lst)
  "Return a random element in lst."
  (if (null lst)
      nil
      (let ((index (random (1- (length lst)))))
        (nth index lst))))

;;point operation
(defun make-point (x y)
  "Create a point x y."
  (cons x y))

(defun x-pos (point)
  "Return the x-axis value of point."
  (car point))

(defun y-pos (point)
  "Return the y-axis value of point."
  (cdr point))

;;players related
(defun marker (player)
  "Returns the marker of the player."
  (if (= player 1) 'O 'X))

(defun the-other-player (this-player)
  "Returns the id of player other than this-player."
  (if (= this-player 1) 2 1))

(defun signal-player (player)
  "Signals player to make a move."
  (format t "It's Player~a's turn~&" player))

;;;---------------------- the board section -------------------------

;; the board and its related operation
(defmacro for-each-point-in-board
  ((x-var y-var board-size &optional (this-block-name nil this-block-
namep)) &body body)
    "Loops through each point in board, binding x-var and y-var to
each coordinates of
   the point and execute body"
   (let ((block-name (if this-block-namep this-block-name (gensym))))
     `(block ,block-name
        (do ((,x-var 0 (1+ ,x-var)))
          ((> ,x-var (1- board-size)) (return-from ,block-name nil))
          (do ((,y-var 0 (1+, y-var)))
            ((> ,y-var (1- ,board-size)) nil)
            ,@body)))))

(let ((board nil)
      (board-size 0)
      (moves (cons nil nil))
      (current-player nil)
      (game-in-progress nil))

  (defun core-dump ()
    (format t "board ~a, size ~a, moves ~a, current-player ~a, game-in-
progress ~a ~&"
            board board-size moves current-player game-in-progress))
  ;;accessors
  (defun is-game-in-progress ()
    "Check wether the game is running."
    game-in-progress)

  (defun examine-board ()
    "Return a copy of the board for examination."
    ;(copy-seq board) ;this doesn't work, try something else later
    board) ;!!!this is a big problem, need to find out how to copy the
board

  (defun size-of-board ()
    "Return the size of the board."
    board-size)

  (defun moves-of-player (player)
    "Return the moves made by player."
    (if (= 1 player)
        (copy-list (car moves))
        (copy-list (cdr moves))))

  (defun previous-move (player)
    "returns the previous move that was made by player."
    (first (moves-of-player player)))

  (defun insert-move (player move)
    "Inserts move made by player into moves."
    (if (= 1 player)
        (push move (car moves))
        (push move (cdr moves))))

  (defun pop-move (player)
    "Pops a move made by player."
    (if (= 1 player)
        (pop (car moves))
        (pop (cdr moves))))

  ;;constructor
  (defun create-board (size)
    "Create a square board with size units in length."
    (setf board-size size)
    (setf board (make-array (list size size) :initial-element nil))
    (setf current-player 1)
    (setf game-in-progress t))

  ;; position related
  (defun in-bound (point)
    "Check if point is within the boundary of the current board."
    (let ((x (x-pos point))
          (y (y-pos point)))
      (and (and (>= x 0) (< x board-size))
           (and (>= y 0) (< y board-size)))))

  (defun position-has-marker (x y marker)
    "Check if the marker at x, y of the baord is of the marker
marker."
    (if (in-bound (make-point x y))
        (eql marker (aref board x y))
        nil))

  (defun empty-position (x y)
    "Check if the position (x,y) is nil"
    (position-has-marker x y nil))

  (defun empty-point (point)
    (empty-position (x-pos point) (y-pos point)))

  (defun marker-at-point (point)
    "Return the element at position specified by point."
    (aref board (x-pos point) (y-pos point)))

  ;;remove this if marker-at-point is defined as a macro
  (defun set-marker-at-point (point value)
    (setf (aref board (x-pos point) (y-pos point)) value))

  ;;remove this if marker-at-point is defined as a macro
  (defsetf marker-at-point set-marker-at-point)

  (defun marker-at (x y)
    (aref board x y))

  (defun filter-board (fn)
    "Returns the list of points in board that satisfies fn."
    (let ((result-list nil))
      (for-each-point-in-board (x y board-size)
        (when (funcall fn x y)
          (push (make-point x y) result-list)))
      (reverse result-list)))

  ;;gui operation
  (defun print-board ()
    "Prints the layout of the board."
    (for-each-point-in-board (x y board-size)
      (if (empty-position x y)
          (format t ". ")
          (format t "~a " (marker-at x y)))
      (when (= y (1- board-size)) (format t "~&"))))

  (defun congratulate ()
    "Congratulates the current player for winning."
    (format t "~a has won. Congratulation!~&" (if (= 1 current-player)
                                                  (car player-names)
                                                  (cdr player-
names))))

  (defun signal-end-of-game ()
    (error 'end-of-game)) ;this is just a signal, not error

  (defun draw-the-game ()
    (format t "The game is a draw, stalemate."))

  ;; game playing operation
  (defun next-player ()
    "Signal the other player to make a move."
    (setf current-player (the-other-player current-player))
    (signal-player current-player))

  (defun insert-marker (point)
    "handles te move by current-player."
    (if (empty-point point)
        (progn
          (insert-move current-player point) ;;moves
          (setf (marker-at-point point) (marker current-player))
          (if (win?)
              (progn
                (congratulate)
                (setf game-in-progress nil)
                (signal-end-of-game))
              (if (draw?)
                  (draw-the-game)
                  (next-player))))
        (error 'Invalid-Position :text point)))

  (defun take-back ()
    "Takes back a move that's done previously by the current player."
    (let* ((his-move (pop-move (the-other-player current-player)))
           (my-move (pop-move current-player)))
      (when (and his-move my-move)
        (setf (marker-at-point his-move) nil))
        (setf (marker-at-point my-move) nil)))

  (defun draw? ()
    "Check if there's any possible move, if there's none, its true."
    (for-each-point-in-board (x y board-size this-block)
      (when (empty-position x y)
        (return-from this-block nil))
      (when (and (>= x (1- board-size)) (>= y (1- board-size))
                 (not (empty-position x y)))
        (return-from this-block t))))

  (defun winning-move? (player this-move range-to-check)
    "Check if player can win the game by making this-move."
    ;just need to check the effect of the current move."
    (let ((his-marker (marker player)))
      (macrolet ((partial-line-test (n (x-init y-init) (x-step y-
step))
                   `(do ((x ,x-init ,x-step)
                         (y ,y-init ,y-step)
                         (result 0))
                      ((or (>= x board-size)
                           (< x 0)
                           (>= y board-size)
                           (< y 0))
                       result)
                      (if (and (position-has-marker x y his-marker)
                               (<= result ,n))
                          (when (not (and (= x x-init) (= y y-init)))
                            (incf result))
                          (return result))))
                 (line-test (n (x-init y-init) (x-step1 y-step1) (x-
step2 y-step2))
                   `(<= ,n (+ (partial-line-test ,n (,x-init ,y-init)
(,x-step1 ,y-step1))
                              (partial-line-test ,n (,x-init ,y-init)
(,x-step2 ,y-step2))))))
        (flet ((vertical-test (x-init y-init n)
                 (line-test n (x-init y-init) (x (- y 1)) (x (+ y
1))))
               (horizontal-test (x-init y-init n)
                 (line-test n (x-init y-init) ((- x 1) y) ((+ x 1)
y)))
               (diagonal-test (x-init y-init n)
                 (or (line-test n (x-init y-init) ((- x 1) (- y 1))
((+ x 1)(+ y 1)))
                     (line-test n (x-init y-init) ((+ x 1) (- y 1))
((- x 1)(+ y 1))))))
          (let ((x (x-pos this-move))
                (y (y-pos this-move))
                (n (1- range-to-check)))
            (or (vertical-test x y n)
              (horizontal-test x y n)
              (diagonal-test x y n)))))))

  (defun win? ()
    "Check if the current player has won just after making his move."
    (winning-move? current-player (previous-move current-player) 3)))

(defun test()
  "test the board"
  (create-board 4)
  (insert-marker (make-point 0 0))
  (insert-marker (make-point 1 1))
  (insert-marker (make-point 0 1))
  (insert-marker (make-point 1 0))
  (insert-marker (make-point 0 2)))

;;;--------------------- the AI-player section
----------------------------
;;utilies for the AI-player
(defmacro find-all-sub-chains (points
                            marker
                            ((x1 x2 y1 y2) sorting-code)
                            ((this-x this-y last-x last-y) comparison-
code))
  "create subroutines that return subchains with a given condition."
  `(let ((sorted-points (sort ,points #'(lambda (point1 point2)
                                          (let ((,x1 (x-pos point1))
                                                (,x2 (x-pos point2))
                                                (,y1 (y-pos point1))
                                                (,y2 (y-pos point2)))
                                            ;sorting code goes here
                                            ,sorting-code)))))
     (labels ((helper (lst final-list acc ,last-x ,last-y)
                (if (null lst)
                    (if (null acc)
                        final-list
                        (if (null (rest acc))
                            (cons (cons ,marker acc) final-
list) ;single element acc
                            (cons (cons ,marker (reverse acc)) final-
list))) ;marker goes here
                    (let* ((this-point (first lst))
                           (,this-x (x-pos this-point))
                           (,this-y (y-pos this-point)))
                      (cond ((null acc) ; implies that last-x and last-
y are also null
                             (helper (rest lst)
                                     final-list
                                     (cons this-point acc) ,this-
x ,this-y))
                            (,comparison-code ;comparison-code, if
true then it belongs to a chain, else its another chain
                                              (helper (rest lst) final-
list (cons this-point acc) ,this-x ,this-y))
                            (t (helper (rest lst)
                                       (if (null (rest acc))
                                           (cons (cons ,marker acc)
final-list) ;single element
                                           (cons (cons ,marker
(reverse acc)) final-list)) ;marker goes here
                                       (cons this-point nil) ,this-
x ,this-y)))))))
       (helper sorted-points nil nil nil nil))))

(defun find-all-h-chains (points)
  "find all horizontal linked chains and return them as a list
  with car 'h, and cdr a list of points sorted from left to right."
  (find-all-sub-chains points
                   'h
                   ((x1 x2 y1 y2)
                    (cond ((< y1 y2) t)
                          ((= y1 y2)
                           (if (< x1 x2) t nil))
                          (t nil)))
                   ((this-x this-y last-x last-y)
                    (and (= this-y last-y) (= 1 (- this-x last-x))))))

(defun find-all-v-chains (points)
  "find all vertical linked chains and return them as a list
  with car 'v, and cdr a list of points sorted from top to bottom."
  (find-all-sub-chains points
                       'v
                       ((x1 x2 y1 y2)
                        (cond ((< x1 x2) t)
                              ((= x1 x2)
                               (if (< y1 y2) t nil))
                              (t nil)))
                       ((this-x this-y last-x last-y)
                        (and (= this-x last-x) (= 1 (- this-y last-
y))))))

(defun find-all-d1-chains (points)
  "find all left->right diagonal linked chains and return them as a
list
  with car 'd1, and cdr a list of points sorted from to right to
bottom left."
  (find-all-sub-chains points
                       'd1
                       ((x1 x2 y1 y2)
                        (cond ((> (- x1 y1) (- x2 y2)) t)
                              ((= (- x1 y1) (- x2 y2))
                               (< (+ x1 y1) (+ x2 y2)))
                              (t nil)))
                       ((this-x this-y last-x last-y)
                        (and (= (- this-x this-y) (- last-x last-y))
                             (= 2 (- (+ this-x this-y)
                                     (+ last-x last-y)))))))

(defun find-all-d2-chains (points)
  "find all right->left diagonal linked chains and return them as a
list
  with car 'd2, and cdr a list of points sorted from top left to
bottom right."
  (find-all-sub-chains points
                       'd2
                       ((x1 x2 y1 y2)
                        (cond ((> (+ x1 y1) (+ x2 y2)) t)
                              ((= (+ x1 y1) (+ x2 y2))
                               (< (- x1 y1) (- x2 y2)))
                              (t nil)))
                       ((this-x this-y last-x last-y)
                        (and (= (+ this-x this-y) (+ last-x last-y))
                             (= 2 (- (- this-x this-y)
                                     (- last-x last-y)))))))

(defun find-all-chains (points)
  "find all the linked chains."
  (let ((all-chains (append (find-all-h-chains points)
                            (find-all-v-chains points)
                            (find-all-d1-chains points)
                            (find-all-d2-chains points))))
    (sort (remove-duplicates all-chains :test #'equal)
          #'(lambda (chain1 chain2) (> (length chain1) (length
chain2))))))

(defun test2 ()
  "test for linked chains"
  (let ((lst (list (cons 0 1) (cons 1 1) (cons 0 2) (cons 1 2) (cons 3
3)
                   (cons 2 3) (cons 0 3) (cons 3 2) (cons 1 3) (cons 3
1)
                   (cons 1 0) (cons 2 0) (cons 2 1) (cons 2 2) (cons 2
3))))
    (find-all-chains lst)))

;;utilites for find-end-points
;;;!!!There's some problems in these codes, that's why the AI-player
is not working properly
(defmacro top (point)
  `(make-point (x-pos ,point) (1+ (y-pos ,point))))

(defmacro bottom (point)
  `(make-point (x-pos ,point) (1- (y-pos ,point))))

(defmacro left (point)
  `(make-point (1- (x-pos ,point)) (y-pos ,point)))

(defmacro right (point)
  `(make-point (1+ (x-pos ,point)) (y-pos ,point)))

(defmacro top-left (point)
  `(make-point (1- (x-pos ,point))
               (1+ (y-pos ,point))))

(defmacro bottom-left (point)
  `(make-point (1+ (x-pos ,point))
               (1- (y-pos ,point))))

(defmacro top-right (point)
  `(make-point (1+ (x-pos ,point))
               (1+ (y-pos ,point))))

(defmacro bottom-right (point)
  `(make-point (1- (x-pos ,point))
               (1- (y-pos ,point))))

(defun length-of-chain (chain)
  "return the length of the chain with the label omitted."
  (length (rest chain)))

(defun find-end-points (chain)
  "return the end points of a chain."
  (let ((type (first chain))
        (head (second chain))
        (tail (car (last chain))))
    (case type
          (nil nil)
          ('v (list (top head) (bottom tail)))
          ('h (list (left head) (right tail)))
          ('d1 (list (top-left head) (bottom-right tail)))
          ('d2 (list (top-right head) (bottom-left tail)))
          (t (error "In function find-end-points(~a): unknown type-
specifier: ~a"
                    chain type)))))

(defun test3 ()
  (mapcar #'find-end-points (test2)))

;;; The AI-player
;;;idea , define functions: find-the-best-move-to-win, find-the-best-
move-to-counter-attack
;;;and then incorporate these into my-insert, well, these surely will
increase the modularity
;;; also, maybe i can use CLOS here to turn this into a class, so that
a sample of machine
;;; to machine match can be deomonstrated.

;;; better still, this can be posted online, and make an interface for
people to write there
;;; own AI-player and then use them to match against each other. With
a server setup to host
;;; the board and print the output the result to each client, such as
the board, the moves,
;;; and other necesssary information, and then the client can accept
these datas and program
;;; their AI in whatever language they use. Sounds interesting right?
(let ((board-size 0)
      (my-id 1)
      (opponent-id 2)
      (my-marker nil)
      (op-marker nil))

  ;;constructor
  (defun initialise-AI-player (size player-id)
    "Initialise the artificial player."
    (setf board-size size)
    (setf my-id player-id)
    (if (= 1 player-id)
        (setf (car player-names) 'Computer)
        (setf (cdr player-names) 'Computer))
    (setf opponent-id (the-other-player player-id))
    (setf my-marker (marker player-id))
    (setf op-marker (marker (the-other-player player-id))))

  (defun my-insert ()
    (let* ((board (examine-board))
           (my-move (previous-move my-id))
           (all-opponent-moves (moves-of-player opponent-id))
           (all-my-moves (moves-of-player my-id)))

        (block done
          (if (= my-id 1) ;i'm player1
            (if (null my-move) ;first move of the game
                (let ((my-position (make-point (random (1- board-
size)) (random (1- board-size)))))
                  (insert-marker my-position)) ;!!!Need Improvement,
maybe in the center is better than this.
                ;a move later in the game, damn, this is where all the
AI comes to.
                (let ((the-chains (find-all-chains all-my-moves)))
                  ;for now, just to check for the longest chain,
                  ;better result could be achieve if considering the
gaps between the chains too,
                  ;but that's going to be harder to implement, maybe
latter,
                  ;for now, i just want to get him working.
                  (dolist (this-chain the-chains)
                    (format t "this-chain: ~a~&" this-chain);remove-
this
                    (let ((possible-moves (find-end-points this-
chain)))
                      (format t "possible-moves: ~a~&" possible-
moves);remove-this
                      (dolist (this-move possible-moves)
                        (format t "this-move: ~a~&" this-move);remove-
this
                        (when (empty-point this-move)
                          (insert-marker this-move)
                          (return-from done)))))))
            ;i'm player2, well try to stop player1 from winning, maybe
at the sametime, try
            ;to win the game, but that's gonna need more brainpower
that i have now,
            ;figure that out later, right now, just make me work.
            (let ((the-chains (find-all-chains all-opponent-
moves)));looks familiar, just C&P from above
              (dolist (this-chain the-chains)
                (format t "this-chain: ~a~&" this-chain);remove-this
                (let ((possible-moves (find-end-points this-chain)))
                  (format t "possible-moves: ~a~&" possible-
moves);remove-this
                  (dolist (this-move possible-moves)
                    (format t "this-move: ~a~&" this-move);remove-this
                    (when (empty-point this-move)
                      (insert-marker this-move)
                      (return-from done)))))))))))

;;;a sample run of the whole game, maybe later will turn this into the
user interface for playing the game

(defun sample-run ()
  (create-board 3)
  (initialise-ai-player 3 1)
  (setf (cdr player-names) 'you)
  (handler-case
    (loop
      (if (draw?)
          (return)
          (progn
            (my-insert)
            (print-board)
            (tagbody
              loop
              (handler-case
                (let ((pos (read)))
                  (insert-marker (make-point (1- (car pos)) (1- (cdr
pos)))))
                (Invalid-position ()
                                  (format t "This is not a valid
position, please try again: ")
                                  (go loop)))))))
    (end-of-game () (print-board))))

Tom

From: Pillsy
Subject: Re: My First Lisp Program
Date: 
Message-ID: <1189004248.257757.276810@o80g2000hse.googlegroups.com>
On Sep 5, 9:16 am, w8 <··········@gmail.com> wrote:
[...]
> I'm just wondering whether the style I am writing is that of Lisp. I
> would be grateful if any one would take a look at it and give me
> suggestion for improvement. Thanks a lot in advance.

Well, this is a lot of code, so this isn't going to be comprehensive.
I'm just pointing out some things I noticed skimming over it.

> (defparameter player-names (cons nil nil))

When defining global variables with DEFVAR or DEFPARAMETER, it's
virtually universal to emphasize their names with *'s, like

(defparameter *player-names* (cons nil nil))

Not only is it nice to point out that you're using global variables,
DEFVAR and DEFPARAMETER also make the global variables
"special" (dynamically scoped). If you aren't aware of this it can be
really surprising.

(defvar *x* 'florble)

(defun foo () *x*)

(let ((*x* 'zorble))
  (foo)) => ZORBLE

> (defun filter (fn lst) ;not used?
>   "Filter lst by a function fn."
>   (let ((acc nil))
>     (dolist (x lst)
>       (let ((val (funcall fn x)))
>         (if val (push val acc))))
>     (reverse acc)))

This function is functionally equivalent to REMOVE-IF-NOT[1], except
that it's less general. I'd just use that instead. If you really want
to write your own, this is the perfect place to use the extended LOOP
macro:

(defun filter (fn list) ; CL has a separate namespace for variables,
			; so LIST is a perfectly good variable name.
  (loop
     :for elt :in list
     :when (funcall fn elt)
       :collect elt))

[1] http://www.lisp.org/HyperSpec/Body/fun_removecm__elete-if-not.html

[...]
> ;;point operation
> (defun make-point (x y)
>   "Create a point x y."
>   (cons x y))

This is fine, but some people prefer using DEFSTRUCT to do this sort
of thing.
[...]
> (defun the-other-player (this-player)
>   "Returns the id of player other than this-player."
>   (if (= this-player 1) 2 1))

Things usually are indexed from 0 in Common Lisp. I find that going
with that particular flow usually makes my life easier.

> ;; the board and its related operation
> (defmacro for-each-point-in-board
>   ((x-var y-var board-size &optional (this-block-name nil this-block-
> namep)) &body body)

Mixing &OPTIONAL and &BODY parameters like this is bad news. It leaves
you open to all sorts of subtle and annoying problems.

>     "Loops through each point in board, binding x-var and y-var to
> each coordinates of
>    the point and execute body"
>    (let ((block-name (if this-block-namep this-block-name (gensym))))
>      `(block ,block-name
>         (do ((,x-var 0 (1+ ,x-var)))
>           ((> ,x-var (1- board-size)) (return-from ,block-name nil))
             ^^^^^^^^^^^^^^^^^^^^^^^^^
This is a bug. You want

(> ,x-var (1- ,board-size))

Without the , to unquote board-size, this will only work by happy
coincidence. Also, you might want to get in the habit breaking control-
structure macros like these into a higher-order function that does
most of the work and a macro that just simplifies the syntax. That way
you can change the implementation of your control structure without
needing to recompile a bunch of stuff. Something like this might be in
order:

(defun traverse-board (board-size &key
		       (on-each (constantly nil))
		       (until (constantly nil)))
  ;; Passing in closures is a good way to defer execution of blocks of
  ;; code, and the idiom of writing a macro to make the process more
  ;; convenient is a popular one.
  (let ((loop-body
	   (lambda (x y)
	     ;; This isn't the fastest possible way of doing things,
	     ;; especially when the ON-EACH or UNTIL arguments aren't
	     ;; present.  If it matters, you can fix that.
	     (let ((untilp (funcall until x y)))
	       (when untilp
		 (return-from traverse-board untilp)))
	     (funcall on-each x y))))
    (dotimes (x board-size)
      ;; Use DOTIMES when you can. It'll save you a lot of fence-post
      ;; errors.
      (dotimes (y board-size)
	(funcall loop-body x y)))))

(defmacro do-board ((x-var y-var board-size) (&optional test-form)
			      &body body)
  `(traverse-board ,board-size
		   :do #'(lambda (,x-var ,y-var)
			   ,@body)
		   :until #'(lambda (,x-var ,y-var)
			      (declare (ignorable ,x-var ,y-var))
			      ,test-form)))

[...]
> (let ((board nil)
>       (board-size 0)
>       (moves (cons nil nil))
>       (current-player nil)
>       (game-in-progress nil))

Closing over these variables is a bit strange. It'll work, but I'd
prefer to use globals or a single data-structure that I can pass as an
argument. It makes it easier to test and play around with (the dynamic
scoping of globals that I mentioned above is particularly useful in
such cases).

Cheers,
Pillsy
From: Thomas A. Russ
Subject: Re: My First Lisp Program
Date: 
Message-ID: <ymi4pi9geqf.fsf@blackcat.isi.edu>
Pillsy <·········@gmail.com> writes:

I agree with all of the snipped parts!  Good advice.


> > ;; the board and its related operation
> > (defmacro for-each-point-in-board
> >   ((x-var y-var board-size &optional (this-block-name nil this-block-
> > namep)) &body body)
> 
> Mixing &OPTIONAL and &BODY parameters like this is bad news. It leaves
> you open to all sorts of subtle and annoying problems.

Actually, not really in the case of this macro.  That is because macros
do destructuring and the &optional is safely inside the parentheses for
the parameters.  There won't be any ambiguity between the &optional and
the &body parts.  Now if the &optional weren't nested, THEN it would
open up problems.

  (defmacro good ((a b c &optional x y z) &body body) ...)
  (defmacro bad  (a b c &optional x y z &body body) ...)

-- 
Thomas A. Russ,  USC/Information Sciences Institute
From: Ken Tilton
Subject: Re: My First Lisp Program
Date: 
Message-ID: <z6ADi.19$vN5.4@newsfe12.lga>
Pillsy wrote:
> On Sep 5, 9:16 am, w8 <··········@gmail.com> wrote:
> [...]
> 
>>I'm just wondering whether the style I am writing is that of Lisp. I
>>would be grateful if any one would take a look at it and give me
>>suggestion for improvement. Thanks a lot in advance.
> 
> 
> Well, this is a lot of code, so this isn't going to be comprehensive.
> I'm just pointing out some things I noticed skimming over it.
> 
> 
>>(defparameter player-names (cons nil nil))
> 
> 
> When defining global variables with DEFVAR or DEFPARAMETER, it's
> virtually universal to emphasize their names with *'s, like
> 
> (defparameter *player-names* (cons nil nil))
> 
> Not only is it nice to point out that you're using global variables,
> DEFVAR and DEFPARAMETER also make the global variables
> "special" (dynamically scoped). If you aren't aware of this it can be
> really surprising.
> 
> (defvar *x* 'florble)
> 
> (defun foo () *x*)
> 
> (let ((*x* 'zorble))
>   (foo)) => ZORBLE
> 
> 
>>(defun filter (fn lst) ;not used?
>>  "Filter lst by a function fn."
>>  (let ((acc nil))

Just (let (acc)... will suffice.
>>    (dolist (x lst)
>>      (let ((val (funcall fn x)))
>>        (if val (push val acc))))
>>    (reverse acc)))

OP: Learn this at once or you will be doomed to writing slow Lisp: you 
just created that list and own all the cons cells exclsuively, so there 
is no need to copy the list: (nreverse acc).

Always know when you can use destructive list operations, then always 
use them.

> 
> 
> This function is functionally equivalent to REMOVE-IF-NOT[1], except
> that it's less general. I'd just use that instead. If you really want
> to write your own, this is the perfect place to use the extended LOOP
> macro:
> 
> (defun filter (fn list) ; CL has a separate namespace for variables,
> 			; so LIST is a perfectly good variable name.
>   (loop
>      :for elt :in list
>      :when (funcall fn elt)
>        :collect elt))

Great, Pillsy, you just broke the OP's function. Let us hope it is 
indeed "not used?". The Op was accumulating the fn return value, not the 
original list element. Ergo:

    (loop for e in list when (funcall fn e) collect it)

This is as far as I read, so...

kt



-- 
http://www.theoryyalgebra.com/

"We are what we pretend to be." -Kurt Vonnegut
From: Pillsy
Subject: Re: My First Lisp Program
Date: 
Message-ID: <1189010500.101141.184330@19g2000hsx.googlegroups.com>
On Sep 5, 11:39 am, Ken Tilton <···········@optonline.net> wrote:
> Pillsy wrote:
[...]
> > (defun filter (fn list) ; CL has a separate namespace for variables,
> >                    ; so LIST is a perfectly good variable name.
> >   (loop
> >      :for elt :in list
> >      :when (funcall fn elt)
> >        :collect elt))

> Great, Pillsy, you just broke the OP's function. Let us hope it is
> indeed "not used?". The Op was accumulating the fn return value, not the
> original list element.

Oops, so he was. Boy is my face red.

Cheers,
Pillsy
From: Timofei Shatrov
Subject: Re: My First Lisp Program
Date: 
Message-ID: <46dfb632.98916424@news.readfreenews.net>
On Wed, 05 Sep 2007 11:39:13 -0400, Ken Tilton <···········@optonline.net> tried
to confuse everyone with this message:

>>>(defun filter (fn lst) ;not used?
>>>  "Filter lst by a function fn."
>>>  (let ((acc nil))
>
>Just (let (acc)... will suffice.
>>>    (dolist (x lst)
>>>      (let ((val (funcall fn x)))
>>>        (if val (push val acc))))
>>>    (reverse acc)))
>
>OP: Learn this at once or you will be doomed to writing slow Lisp: you 
>just created that list and own all the cons cells exclsuively, so there 
>is no need to copy the list: (nreverse acc).
>
>Always know when you can use destructive list operations, then always 
>use them.
>

I think this is called premature optimization.

-- 
|Don't believe this - you're not worthless              ,gr---------.ru
|It's us against millions and we can't take them all... |  ue     il   |
|But we can take them on!                               |     @ma      |
|                       (A Wilhelm Scream - The Rip)    |______________|
From: Ken Tilton
Subject: Re: My First Lisp Program
Date: 
Message-ID: <kESDi.19$Tu5.7@newsfe12.lga>
Timofei Shatrov wrote:
> On Wed, 05 Sep 2007 11:39:13 -0400, Ken Tilton <···········@optonline.net> tried
> to confuse everyone with this message:
> 
> 
>>>>(defun filter (fn lst) ;not used?
>>>> "Filter lst by a function fn."
>>>> (let ((acc nil))
>>
>>Just (let (acc)... will suffice.
>>
>>>>   (dolist (x lst)
>>>>     (let ((val (funcall fn x)))
>>>>       (if val (push val acc))))
>>>>   (reverse acc)))
>>
>>OP: Learn this at once or you will be doomed to writing slow Lisp: you 
>>just created that list and own all the cons cells exclsuively, so there 
>>is no need to copy the list: (nreverse acc).
>>
>>Always know when you can use destructive list operations, then always 
>>use them.
>>
> 
> 
> I think this is called premature optimization.
> 

No, as Andreas already intimated, optimization is something different. 
Optimization is writing really brittle code and you know it but you have 
to because you are getting performance out of knowing exactly how 
something works but boy if that changes are you screwed, but that's OK, 
we know it, we need the performance because we have /seen/ the thing bog 
down so we are not being premature and we don't do this too soon because 
it is brittle and you do not want to be forever reworking it as you 
refactor during normal development.

Look at it another way. Do you (or anyone) know that it is OK to use 
nreverse on that local variable? If not, you (or anyone) just do not 
know Lisp. But I will assume you know nreverse is OK. If you know 
nreverse is OK, why would you use reverse?

The only controversial thing I am saying is that noobs should not defer 
mastering cons structures. I would defend that position by pointing out 
that even a noob can quickly ened up writing inefficient code if they 
decide to tackle the right learning exercise, such as eight queens. And 
in any case one of the big wins of Lisp is lists, and if one is not 
fluent in consology Lisp can be a little hard to program when things get 
even a little interesting.

kt

-- 
http://www.theoryyalgebra.com/

"We are what we pretend to be." -Kurt Vonnegut
From: Andreas Thiele
Subject: Re: My First Lisp Program
Date: 
Message-ID: <fbohaa$up8$03$1@news.t-online.com>
"Timofei Shatrov" <····@mail.ru> schrieb im Newsbeitrag ······················@news.readfreenews.net...
> On Wed, 05 Sep 2007 11:39:13 -0400, Ken Tilton <···········@optonline.net> tried
> ...
>>OP: Learn this at once or you will be doomed to writing slow Lisp: you
>>just created that list and own all the cons cells exclsuively, so there
>>is no need to copy the list: (nreverse acc).
>>
>>Always know when you can use destructive list operations, then always
>>use them.
>>
>
> I think this is called premature optimization.
> ...

Not at all, it's just knowing your language idioms.

Andreas
From: w8
Subject: Re: My First Lisp Program
Date: 
Message-ID: <1189005544.412364.55090@o80g2000hse.googlegroups.com>
On Sep 5, 10:57 pm, Pillsy <·········@gmail.com> wrote:
> On Sep 5, 9:16 am, w8 <··········@gmail.com> wrote:
> [...]
>
> > I'm just wondering whether the style I am writing is that of Lisp. I
> > would be grateful if any one would take a look at it and give me
> > suggestion for improvement. Thanks a lot in advance.
>
> Well, this is a lot of code, so this isn't going to be comprehensive.
> I'm just pointing out some things I noticed skimming over it.
>
> > (defparameter player-names (cons nil nil))
>
> When defining global variables with DEFVAR or DEFPARAMETER, it's
> virtually universal to emphasize their names with *'s, like
>
> (defparameter *player-names* (cons nil nil))
>
> Not only is it nice to point out that you're using global variables,
> DEFVAR and DEFPARAMETER also make the global variables
> "special" (dynamically scoped). If you aren't aware of this it can be
> really surprising.
>
> (defvar *x* 'florble)
>
> (defun foo () *x*)
>
> (let ((*x* 'zorble))
>   (foo)) => ZORBLE
>
> > (defun filter (fn lst) ;not used?
> >   "Filter lst by a function fn."
> >   (let ((acc nil))
> >     (dolist (x lst)
> >       (let ((val (funcall fn x)))
> >         (if val (push val acc))))
> >     (reverse acc)))
>
> This function is functionally equivalent to REMOVE-IF-NOT[1], except
> that it's less general. I'd just use that instead. If you really want
> to write your own, this is the perfect place to use the extended LOOP
> macro:
>
> (defun filter (fn list) ; CL has a separate namespace for variables,
>                         ; so LIST is a perfectly good variable name.
>   (loop
>      :for elt :in list
>      :when (funcall fn elt)
>        :collect elt))
>
> [1]http://www.lisp.org/HyperSpec/Body/fun_removecm__elete-if-not.html
>
> [...]
>
> > ;;point operation
> > (defun make-point (x y)
> >   "Create a point x y."
> >   (cons x y))
>
> This is fine, but some people prefer using DEFSTRUCT to do this sort
> of thing.
> [...]
>
> > (defun the-other-player (this-player)
> >   "Returns the id of player other than this-player."
> >   (if (= this-player 1) 2 1))
>
> Things usually are indexed from 0 in Common Lisp. I find that going
> with that particular flow usually makes my life easier.
>
> > ;; the board and its related operation
> > (defmacro for-each-point-in-board
> >   ((x-var y-var board-size &optional (this-block-name nil this-block-
> > namep)) &body body)
>
> Mixing &OPTIONAL and &BODY parameters like this is bad news. It leaves
> you open to all sorts of subtle and annoying problems.
>
> >     "Loops through each point in board, binding x-var and y-var to
> > each coordinates of
> >    the point and execute body"
> >    (let ((block-name (if this-block-namep this-block-name (gensym))))
> >      `(block ,block-name
> >         (do ((,x-var 0 (1+ ,x-var)))
> >           ((> ,x-var (1- board-size)) (return-from ,block-name nil))
>
>              ^^^^^^^^^^^^^^^^^^^^^^^^^
> This is a bug. You want
>
> (> ,x-var (1- ,board-size))
>
> Without the , to unquote board-size, this will only work by happy
> coincidence. Also, you might want to get in the habit breaking control-
> structure macros like these into a higher-order function that does
> most of the work and a macro that just simplifies the syntax. That way
> you can change the implementation of your control structure without
> needing to recompile a bunch of stuff. Something like this might be in
> order:
>
> (defun traverse-board (board-size &key
>                        (on-each (constantly nil))
>                        (until (constantly nil)))
>   ;; Passing in closures is a good way to defer execution of blocks of
>   ;; code, and the idiom of writing a macro to make the process more
>   ;; convenient is a popular one.
>   (let ((loop-body
>            (lambda (x y)
>              ;; This isn't the fastest possible way of doing things,
>              ;; especially when the ON-EACH or UNTIL arguments aren't
>              ;; present.  If it matters, you can fix that.
>              (let ((untilp (funcall until x y)))
>                (when untilp
>                  (return-from traverse-board untilp)))
>              (funcall on-each x y))))
>     (dotimes (x board-size)
>       ;; Use DOTIMES when you can. It'll save you a lot of fence-post
>       ;; errors.
>       (dotimes (y board-size)
>         (funcall loop-body x y)))))
>
> (defmacro do-board ((x-var y-var board-size) (&optional test-form)
>                               &body body)
>   `(traverse-board ,board-size
>                    :do #'(lambda (,x-var ,y-var)
>                            ,@body)
>                    :until #'(lambda (,x-var ,y-var)
>                               (declare (ignorable ,x-var ,y-var))
>                               ,test-form)))
>
> [...]
>
> > (let ((board nil)
> >       (board-size 0)
> >       (moves (cons nil nil))
> >       (current-player nil)
> >       (game-in-progress nil))
>
> Closing over these variables is a bit strange. It'll work, but I'd
> prefer to use globals or a single data-structure that I can pass as an
> argument. It makes it easier to test and play around with (the dynamic
> scoping of globals that I mentioned above is particularly useful in
> such cases).
>
> Cheers,
> Pillsy

Thank you, that's really helpful. I'll try to improve on that. I am
currently using Cusp on Windows, and it occasionally crushes, it has a
lot of good features, but every time i exist Eclipse, it left behind
multiple SBCL processes running which really takes up resources and i
have to manually shut them down in the Task manager. I am wondering if
there are any non-commercial lisp IDE that works well on Windows.
Thanks again.

Tom
From: ·············@gmail.com
Subject: Re: My First Lisp Program
Date: 
Message-ID: <1189006246.751356.115890@o80g2000hse.googlegroups.com>
On Sep 5, 11:19 am, w8 <··········@gmail.com> wrote:
> On Sep 5, 10:57 pm, Pillsy <·········@gmail.com> wrote:
>
>
>
> > On Sep 5, 9:16 am, w8 <··········@gmail.com> wrote:
> > [...]
>
> > > I'm just wondering whether the style I am writing is that of Lisp. I
> > > would be grateful if any one would take a look at it and give me
> > > suggestion for improvement. Thanks a lot in advance.
>
> > Well, this is a lot of code, so this isn't going to be comprehensive.
> > I'm just pointing out some things I noticed skimming over it.
>
> > > (defparameter player-names (cons nil nil))
>
> > When defining global variables with DEFVAR or DEFPARAMETER, it's
> > virtually universal to emphasize their names with *'s, like
>
> > (defparameter *player-names* (cons nil nil))
>
> > Not only is it nice to point out that you're using global variables,
> > DEFVAR and DEFPARAMETER also make the global variables
> > "special" (dynamically scoped). If you aren't aware of this it can be
> > really surprising.
>
> > (defvar *x* 'florble)
>
> > (defun foo () *x*)
>
> > (let ((*x* 'zorble))
> >   (foo)) => ZORBLE
>
> > > (defun filter (fn lst) ;not used?
> > >   "Filter lst by a function fn."
> > >   (let ((acc nil))
> > >     (dolist (x lst)
> > >       (let ((val (funcall fn x)))
> > >         (if val (push val acc))))
> > >     (reverse acc)))
>
> > This function is functionally equivalent to REMOVE-IF-NOT[1], except
> > that it's less general. I'd just use that instead. If you really want
> > to write your own, this is the perfect place to use the extended LOOP
> > macro:
>
> > (defun filter (fn list) ; CL has a separate namespace for variables,
> >                         ; so LIST is a perfectly good variable name.
> >   (loop
> >      :for elt :in list
> >      :when (funcall fn elt)
> >        :collect elt))
>
> > [1]http://www.lisp.org/HyperSpec/Body/fun_removecm__elete-if-not.html
>
> > [...]
>
> > > ;;point operation
> > > (defun make-point (x y)
> > >   "Create a point x y."
> > >   (cons x y))
>
> > This is fine, but some people prefer using DEFSTRUCT to do this sort
> > of thing.
> > [...]
>
> > > (defun the-other-player (this-player)
> > >   "Returns the id of player other than this-player."
> > >   (if (= this-player 1) 2 1))
>
> > Things usually are indexed from 0 in Common Lisp. I find that going
> > with that particular flow usually makes my life easier.
>
> > > ;; the board and its related operation
> > > (defmacro for-each-point-in-board
> > >   ((x-var y-var board-size &optional (this-block-name nil this-block-
> > > namep)) &body body)
>
> > Mixing &OPTIONAL and &BODY parameters like this is bad news. It leaves
> > you open to all sorts of subtle and annoying problems.
>
> > >     "Loops through each point in board, binding x-var and y-var to
> > > each coordinates of
> > >    the point and execute body"
> > >    (let ((block-name (if this-block-namep this-block-name (gensym))))
> > >      `(block ,block-name
> > >         (do ((,x-var 0 (1+ ,x-var)))
> > >           ((> ,x-var (1- board-size)) (return-from ,block-name nil))
>
> >              ^^^^^^^^^^^^^^^^^^^^^^^^^
> > This is a bug. You want
>
> > (> ,x-var (1- ,board-size))
>
> > Without the , to unquote board-size, this will only work by happy
> > coincidence. Also, you might want to get in the habit breaking control-
> > structure macros like these into a higher-order function that does
> > most of the work and a macro that just simplifies the syntax. That way
> > you can change the implementation of your control structure without
> > needing to recompile a bunch of stuff. Something like this might be in
> > order:
>
> > (defun traverse-board (board-size &key
> >                        (on-each (constantly nil))
> >                        (until (constantly nil)))
> >   ;; Passing in closures is a good way to defer execution of blocks of
> >   ;; code, and the idiom of writing a macro to make the process more
> >   ;; convenient is a popular one.
> >   (let ((loop-body
> >            (lambda (x y)
> >              ;; This isn't the fastest possible way of doing things,
> >              ;; especially when the ON-EACH or UNTIL arguments aren't
> >              ;; present.  If it matters, you can fix that.
> >              (let ((untilp (funcall until x y)))
> >                (when untilp
> >                  (return-from traverse-board untilp)))
> >              (funcall on-each x y))))
> >     (dotimes (x board-size)
> >       ;; Use DOTIMES when you can. It'll save you a lot of fence-post
> >       ;; errors.
> >       (dotimes (y board-size)
> >         (funcall loop-body x y)))))
>
> > (defmacro do-board ((x-var y-var board-size) (&optional test-form)
> >                               &body body)
> >   `(traverse-board ,board-size
> >                    :do #'(lambda (,x-var ,y-var)
> >                            ,@body)
> >                    :until #'(lambda (,x-var ,y-var)
> >                               (declare (ignorable ,x-var ,y-var))
> >                               ,test-form)))
>
> > [...]
>
> > > (let ((board nil)
> > >       (board-size 0)
> > >       (moves (cons nil nil))
> > >       (current-player nil)
> > >       (game-in-progress nil))
>
> > Closing over these variables is a bit strange. It'll work, but I'd
> > prefer to use globals or a single data-structure that I can pass as an
> > argument. It makes it easier to test and play around with (the dynamic
> > scoping of globals that I mentioned above is particularly useful in
> > such cases).
>
> > Cheers,
> > Pillsy
>
> Thank you, that's really helpful. I'll try to improve on that. I am
> currently using Cusp on Windows, and it occasionally crushes, it has a
> lot of good features, but every time i exist Eclipse, it left behind
> multiple SBCL processes running which really takes up resources and i
> have to manually shut them down in the Task manager. I am wondering if
> there are any non-commercial lisp IDE that works well on Windows.
> Thanks again.
>
> Tom

If you know emacs, some people swear by SLIME  (I use it and
occasionally swear at it).  The setup was not so hard (considering
that even I managed to figure it out).

Mirko
From: Pillsy
Subject: Re: My First Lisp Program
Date: 
Message-ID: <1189010929.940991.229480@g4g2000hsf.googlegroups.com>
On Sep 5, 11:19 am, w8 <··········@gmail.com> wrote:
[...]
> I am wondering if there are any non-commercial lisp IDE that works
> well on Windows.

Unless you have a serious ideological commitment to not using
commercial software, or are already entranced with Emacs (in which
case SLIME is the thing for you), I would recommend downloading the
trial version of either Allegro Common Lisp[1] or LispWorks[2]. Once
you learn the language, you'll have a better idea of which
implementation best suits your needs. I switched a couple times in the
beginning myself.

Cheers,
Pillsy

[1] http://www.franz.com/downloads/
[2] http://www.lispworks.com/
From: Daniel Leidisch
Subject: Re: My First Lisp Program
Date: 
Message-ID: <87veapyvec.fsf@zeus.home>
w8 <··········@gmail.com> writes:

> Thank you, that's really helpful. I'll try to improve on that. I am
> currently using Cusp on Windows, and it occasionally crushes, it has a
> lot of good features, but every time i exist Eclipse, it left behind
> multiple SBCL processes running which really takes up resources and i
> have to manually shut them down in the Task manager. I am wondering if
> there are any non-commercial lisp IDE that works well on Windows.

I think the most used free Lisp environent is Emacs + Slime:

http://www.gnu.org/software/emacs/
http://common-lisp.net/project/slime/

I am using this on Linux and FreeBSD, but I do not think that there
will be any Problems on Windows. There are also Edi Weitz' starter
pack, which is often recommended. 

http://weitz.de/starter-pack/

I have not used it by myself, but maybe it would a good solution for
your needs, but it uses LispWorks, which is not “free”.

Then there is “Lisp in a Box”, which bundels Lisp implementation,
Slime and Emacs.

http://common-lisp.net/project/lispbox/#windows

When I started, I was not aware of those possibilities; they could
have spared me some confusion ;)

HTH

dhl
From: Sergey Kolos
Subject: Re: My First Lisp Program
Date: 
Message-ID: <1189365058.298925.189660@57g2000hsv.googlegroups.com>
> currently using Cusp on Windows, and it occasionally crushes,

I am one of the developers of Cusp. It was quite stable with what I
do, but it definitely has bunch of bugs needed to be fixed. If you
could describe how Cusp crashes at our group: http://groups.google.com/group/cusp-development,
we will work hard to resolve the issues.

Thank you for trying Cusp.
From: dpapathanasiou
Subject: Re: My First Lisp Program
Date: 
Message-ID: <1189017090.032174.186090@50g2000hsm.googlegroups.com>
> I'm a novice to Common Lisp, just finished reading the first half of
> Practical Common Lisp and managed to hack out a tic-tac-toe program.
> I'm just wondering whether the style I am writing is that of Lisp. I
> would be grateful if any one would take a look at it and give me
> suggestion for improvement. Thanks a lot in advance.

Take a look at the Othello game chapter in Peter Norvig's "Paradigms
of Artificial Intelligence Programming" (a.k.a. "PAIP").

It's a great example of design and coding style, in the same context
as your tic-tac-toe program.