··················@gmail.com wrote:
> Recently I've been doing some problems from http://projecteuler.net/ to
> teach myself lisp. This is my first experience with lisp, bottom up
> programming, and functional programming. I'm curious if I have the
> right idea with my solution to this problem
> http://projecteuler.net/index.php?section=problems&id=54. I got the
> right answer, but I'm curious how close my lisp style is. I'd
> appreciate any feedback.
>
> -Ben Zimmerman
>
> I run find-player1-wins with the path of poker.txt to get the answer.
>
Hey Ben,
Congratulations on jumping in and getting your hands dirty! The only
way to learn is by doing...
I can't comment on your algorithm, but the following things jump out
reading over your code:
firstly, your code would be easier to read and probubly more efficient
if you converted the data from the text file to a more meaningful form
(like structures or classes) so that instead of calling (subseq (cadr
(car hand)) 0 1) or something you could call (card-suit (first-card
hand))) and someone reading the code would be able to make more sense
of it
> (defun simplify-card (card)
> "returns a list containing a number representing the card and a
> letter
> representing the suit"
> (let ((value (subseq card 0 1)) (suit (subseq card 1 2)))
> (cond
> ((equal "T" value) (list 10 suit))
> ((equal "J" value) (list 11 suit))
> ((equal "Q" value) (list 12 suit))
> ((equal "K" value) (list 13 suit))
> ((equal "A" value) (list 14 suit))
> (t (list (parse-integer value) suit)))))
if you extracted the character from CARD with CHAR instead of a
single-element string with SUBSEQ string you could use the lisp form
CASE
(defun simplify-card (card)
(let ((v (char card 0)))
(list (case v
(#\T 10)
(#\J 11)
(#\Q 12)
(#\K 13)
(#\A 14)
(otherwise (parse-integer v)))
(subseq card 1 2))))
>
> (defun simplify-hand (hand)
> "returns a list of all cards in simplify-card format"
> (if (= 0 (length hand))
> nil
> (cons (simplify-card
> (string-trim " " (subseq hand 0 3)))
> (simplify-hand (subseq (string-trim " " hand) 2)))))
this might be more readible as
(defun simplify-hand (hand
(unless (= 0 (length hand))
(cons (simplify-car (string-trim " " (subseq hand 0 3)))
(simplify-hand (subseq (string-trim " " hand) 2))))
>
> (defun sort-hand (hand)
> (sort (simplify-hand hand) (lambda (card1 card2) (< (car card1)
> (car card2)))))
>
this could be
(defun sort-hand (hand)
(sort (simplify-hand hand) #'< :key #'first)))
> (defun ace-first (hand)
> "puts the fifth card in the hand first and subtracts 12 from its
> value.
> used for straights where the A counts as a 1"
> (cons (list (- (car (fifth hand)) 12) (cadr (fifth hand)))
> (cons (first hand)
> (cons (second hand)
> (cons (third hand)
> (cons (fourth hand) nil))))))
>
first off
(cons a (cons b (cons c (cons d nil))))
is the same as
(list a b c d)
secondly, this could have just been
(list* (list (- (car (fifth hand)) 12) (cadr (fifth hand)))
(butlast hand))
see my first comment about how confused I am about what (car (fifth
hand)) or (cadr (fifth hand)) could possibly be refering to...
> (defun count-kind (hand)
> (loop for card in hand
> collect (let ((count 0))
> (loop for (tmp-card) in hand
> do (if (= (car card) tmp-card)
> (setf count (+ count 1))))
> (list count (car card)))))
>
I don't condone the use of the extended LOOP macro... but most do...
you're choice...
also, (setf count (+ count 1)) could just be (incf count)
> (defun four-of-a-kind (hand)
> (let ((four (find-if (lambda (x) (equal (car x) 4))
> (count-kind hand))))
> (cond (four (list 8 (cadr four))))))
(defun four-of-a-kind (hand)
(let ((four (assoc 4 (count-kind hand) :key #'first)))
(when four
(list 8 (cadr four)))))
>
> (defun full-house (hand)
> (let ((counted-hand (count-kind hand)))
> (let ((three (find-if (lambda (x) (equal (car x) 3)) counted-hand))
> (two (find-if (lambda (x) (equal (car x) 2)) counted-hand)))
> (cond ((and two three) (list 7 (cadr three)))))))
>
(defun full-house (hand)
(let* ((counted-hand (count-kind hand))
(three (assoc 3 counted-hand))
(two (assoc 2 counted-hand)))
...
anyways, good luck to you
hth
Nick
········@gmail.com wrote:
> I don't condone the use of the extended LOOP macro...
Good to see the noob getting a nice range of feedback on this issue. But
I am concerned. Condone means "excuse, overlook, or make allowances for;
be lenient with". If you are not doing that, um, should I be watching my
back the next time I head down to the pub?
:)
kzo (former anti-loop bigot)
--
The Dalai Lama gets the same crap all the time.
-- Kenny Tilton on c.l.l when accused of immodesty
··················@gmail.com wrote:
> Recently I've been doing some problems from http://projecteuler.net/ to
> teach myself lisp. This is my first experience with lisp, bottom up
> programming, and functional programming. I'm curious if I have the
> right idea with my solution to this problem
> http://projecteuler.net/index.php?section=problems&id=54. I got the
> right answer, but I'm curious how close my lisp style is. I'd
> appreciate any feedback.
How about a few lines of the input? Anyway...
Why are you using read-line instead of read? You would get integers
parsed automatically and symbols instead of strings, which is the first
suggestion I have towards more lispy-ness (symbols are a lot nicer to
work with than strings).
>
> -Ben Zimmerman
>
> I run find-player1-wins with the path of poker.txt to get the answer.
>
> (defun simplify-card (card)
> "returns a list containing a number representing the card and a
> letter
> representing the suit"
> (let ((value (subseq card 0 1)) (suit (subseq card 1 2)))
> (cond
> ((equal "T" value) (list 10 suit))
> ((equal "J" value) (list 11 suit))
> ((equal "Q" value) (list 12 suit))
> ((equal "K" value) (list 13 suit))
> ((equal "A" value) (list 14 suit))
> (t (list (parse-integer value) suit)))))
I'd rather see (in Lisp or assembler):
(list (if (numberp (car card))
(car card) (+ 10 (position (car card) '(T J Q K A)))
(cadr card))
btw, if yer gonna be writing a lot of code, I'd like to see:
(defun card-suit (c) (second c))
...etc... instead of a bunch of cars and cdrs throughout the code. In
Lisp or assembler. :)
>
> (defun simplify-hand (hand)
> "returns a list of all cards in simplify-card format"
> (if (= 0 (length hand))
> nil
> (cons (simplify-card
> (string-trim " " (subseq hand 0 3)))
> (simplify-hand (subseq (string-trim " " hand) 2)))))
If we can lose the string thing, this becomes:
(mapcar 'simplify-card hand)
>
> (defun sort-hand (hand)
> (sort (simplify-hand hand) (lambda (card1 card2) (< (car card1)
> (car card2)))))
(sort .... '< :key 'car)
or, again, (sort ... '< :key 'card-value)
>
> (defun straightp (hand)
> (if (null (cdr hand))
> t
> (and (= (caar hand) (- (caadr hand) 1)) (straightp (cdr hand)))))
Not bad, except of course for the caar/caadr thing. Lisniks have an
irrational bias against recursion where iteration will do, so if they
turn on you just come back with:
(loop for (c1 c2) on hand
unless c2 return t
unless <in order> return nil)
yes, you really should learn LOOP. (I see below you have started.)
>
> (defun ace-first (hand)
> "puts the fifth card in the hand first and subtracts 12 from its
> value.
> used for straights where the A counts as a 1"
> (cons (list (- (car (fifth hand)) 12) (cadr (fifth hand)))
> (cons (first hand)
> (cons (second hand)
> (cons (third hand)
> (cons (fourth hand) nil))))))
Check out butlast.
>
> (defun straight (hand)
> "returns the number of the high card if the hand is a straight else
> nil"
> (cond
> ((straightp hand)
> (list 5 (car (fifth hand))))
> ((straightp (ace-first hand))
> (list 5 (car (fourth hand))))))
>
> (defun flushp (hand)
> (if (null (cdr hand))
> t
> (and (equal (cadar hand) (cadadr hand)) (flushp (cdr hand)))))
That "sells" sequence, when really you want to see if they are all the same:
(every (lambda (c-other)
(eq (card-suit (car hand)) (card-suit c-other))
(cdr hand))
Oh, you can use EQ for any lisp object other than characters or numbers,
and you can use = for numbers. Save equal for lists, eql will serve
generally for chars, numbers, and lisp objects.
>
> (defun flush (hand)
> "returns the number of the high card if the hand is a flush else
> nil"
> (if (flushp hand)
> (list 6 (car (fifth hand)))
> nil))
I like: (when (flushp) <value>)
"else nil" takes care of itself.
Anal types like to make things explicit, manic-depressives like me like
to reduce code clutter so we do not get confused.
>
> (defun straight-flush (hand)
> "returns the number of the high card if the hand is a straight
> flush else nil"
> (if (and (flush hand) (straight hand))
> (list 9 (cadr (straight hand)))
> nil))
>
> (defun count-kind (hand)
> (loop for card in hand
> collect (let ((count 0))
> (loop for (tmp-card) in hand
> do (if (= (car card) tmp-card)
> (setf count (+ count 1))))
> (list count (car card)))))
(list (loop ... counting <test>) (car card))
>
> (defun four-of-a-kind (hand)
> (let ((four (find-if (lambda (x) (equal (car x) 4))
> (count-kind hand))))
> (cond (four (list 8 (cadr four))))))
>
> (defun full-house (hand)
> (let ((counted-hand (count-kind hand)))
> (let ((three (find-if (lambda (x) (equal (car x) 3)) counted-hand))
> (two (find-if (lambda (x) (equal (car x) 2)) counted-hand)))
> (cond ((and two three) (list 7 (cadr three)))))))
>
> (defun three-of-a-kind (hand)
> (let ((three (find-if (lambda (x) (equal (car x) 3))
> (count-kind hand))))
> (cond (three (list 4 (cadr three))))))
>
> (defun two-pair (hand)
> (let ((counted-hand (count-kind hand)))
> (let ((one (second (high-card hand)))
> (two1 (find-if (lambda (x) (equal (car x) 2)) (subseq
> counted-hand 0 2)))
> (two2 (find-if (lambda (x) (equal (car x) 2)) (subseq
> counted-hand 3 5))))
> (cond ((and two1 two2) (list 3 (cadr two2) (cadr two1) one))))))
>
> (defun pair (hand)
> (let ((counted-hand (count-kind hand)))
> (let ((two (find-if (lambda (x) (equal (car x) 2)) counted-hand)))
(find 2 .... :key 'card-value)
> (cond (two (list 2 (cadr two) (second (high-card hand))))))))
>
> (defun high-card (hand)
> "finds the high non-paired card from a counted hand"
> (let ((counted-hand (count-kind hand)) (high 0))
> (loop for (count value) in counted-hand
> do (if (and (equal count 1) (> value high))
> (setf high value)))
> (if (> high 0) (list 1 high) nil)))
(loop for (count value) in (count-kind hand)
when (= count 1)
maximize value)
>
> (defun rank-hand (hand)
> (let ((hand (sort-hand hand)))
> (cond
> ((straight-flush hand))
> ((four-of-a-kind hand))
> ((full-house hand))
> ((flush hand))
> ((straight hand))
> ((three-of-a-kind hand))
> ((two-pair hand))
> ((pair hand))
> ((high-card hand)))))
That is OR. And a tad redundant.
Try (some (lambda (ranker) (funcall ranker hand))
'(straight-flush four-of-a-kind....))
Or (loop for ranker in '(straight-flush....)
thereis (funcall ranker hand))
>
> (defun player1-winp (hands)
> (let ((hand1 (rank-hand (subseq hands 0 14)))
> (hand2 (rank-hand (subseq hands 15 29))))
> (loop for x in hand1
> for y in hand2
> when (> x y) return t
> when (> y x) return nil)))
>
> (defun find-player1-wins (file)
> (let ((wins 0))
> (loop for hands in (read-whole-file file)
> do (if (player1-winp hands)
> (setf wins (+ wins 1))))
> wins))
(count-if 'player-winp ...)
>
> (defun read-whole-file (name)
> (let ((output nil))
> (with-open-file (stream name :direction :input)
> (do ((line (read-line stream nil)
> (read-line stream nil)))
> ((null line))
> (setf output (cons line output))))
> output))
>
Caveats: everything above was typed into the newsreader, ergo nowhere
near tested and parens prob do not even match. And I skipped around,
cherry-picking.
meanwhile, am I the only one in the world not playing poker?
:)
kenzo
--
The Dalai Lama gets the same crap all the time.
-- Kenny Tilton on c.l.l when accused of immodesty
··················@gmail.com writes:
> Recently I've been doing some problems from http://projecteuler.net/ to
> teach myself lisp. This is my first experience with lisp, bottom up
> programming, and functional programming. I'm curious if I have the
> right idea with my solution to this problem
> http://projecteuler.net/index.php?section=problems&id=54. I got the
> right answer, but I'm curious how close my lisp style is. I'd
> appreciate any feedback.
>
> -Ben Zimmerman
>
> I run find-player1-wins with the path of poker.txt to get the answer.
>
> (defun simplify-card (card)
> "returns a list containing a number representing the card and a
> letter
> representing the suit"
> (let ((value (subseq card 0 1)) (suit (subseq card 1 2)))
> (cond
> ((equal "T" value) (list 10 suit))
> ((equal "J" value) (list 11 suit))
> ((equal "Q" value) (list 12 suit))
> ((equal "K" value) (list 13 suit))
> ((equal "A" value) (list 14 suit))
> (t (list (parse-integer value) suit)))))
You need to abstract away!
Don't use subseq. Don't assume a card is a sequence!
(let ((value (card-value card))
(suit (card-suit card)))
)
Wouldn't it be simplier to keep the value of the card in numerical form?
You can convert between the numbers and the names when doing I/O.
Then you wouldn't need this simplify-card...
(defmethod render-card ((card card) (display stream))
(let ((value (card-value card))
(suit (card-suit card)))
(format display "~A~A"
(aref "--23456789TJQKA" value)
(cdr (assoc suit '((s . ♠) (c . ♣) (h . ♥) (d . ♦)))))))
(render-card (make-card :suit 't :value 11) *standard-output*)
J♣
> (defun simplify-hand (hand)
> "returns a list of all cards in simplify-card format"
> (if (= 0 (length hand))
> nil
> (cons (simplify-card
> (string-trim " " (subseq hand 0 3)))
> (simplify-hand (subseq (string-trim " " hand) 2)))))
(= 0 (length list))
can be written: (zerop (length list))
or: (null list)
the later is O(1), the formers are O(length(list)).
(if cond nil something)
can be written: (unless cond something)
with the added advantage that unless takes a body, while if takes only
one expression in each branch.
Same comment, what about keeping the hands in lists instead of strings?
Unless you're processing words, strings are probably better reserved to I/O.
> (defun sort-hand (hand)
> (sort (simplify-hand hand) (lambda (card1 card2) (< (car card1)
> (car card2)))))
(defun card-lessp (a b) ...)
(sort hand (function card-lessp))
> (defun straightp (hand)
> (if (null (cdr hand))
> t
> (and (= (caar hand) (- (caadr hand) 1)) (straightp (cdr hand)))))
Here is my implementation of straightp (I used a rule that accepts the
value of ace as either 1 or 14 in straights):
(defun straightp (h)
(flet ((consecutivep (h)
(apply (function =) 1
(mapcar (lambda (a b) (mod (- (card-weight a) (card-weight b))
*nnums*)) (rest h) h))))
(or (consecutivep h) (consecutivep (cons (car (last h)) (butlast h))))))
> (defun ace-first (hand)
> "puts the fifth card in the hand first and subtracts 12 from its
> value.
> used for straights where the A counts as a 1"
> (cons (list (- (car (fifth hand)) 12) (cadr (fifth hand)))
> (cons (first hand)
> (cons (second hand)
> (cons (third hand)
> (cons (fourth hand) nil))))))
(cons a (cons b ... (cons z nil)))
is better written (list a b ... z)
> (defun straight (hand)
> "returns the number of the high card if the hand is a straight else
> nil"
> (cond
> ((straightp hand)
> (list 5 (car (fifth hand))))
> ((straightp (ace-first hand))
> (list 5 (car (fourth hand))))))
>
> (defun flushp (hand)
> (if (null (cdr hand))
> t
> (and (equal (cadar hand) (cadadr hand)) (flushp (cdr hand)))))
> [...]
> (defun rank-hand (hand)
> (let ((hand (sort-hand hand)))
> (cond
> ((straight-flush hand))
> ((four-of-a-kind hand))
> ((full-house hand))
> ((flush hand))
> ((straight hand))
> ((three-of-a-kind hand))
> ((two-pair hand))
> ((pair hand))
> ((high-card hand)))))
This is a lot of code. What about if we could just compute a score of
a hand, and compare the scores?
Here is what I have, unfinished and to be refactored (we are doing
almost the same thing in each clause, and I've been computing the
offsets in my head, this is a job for a macro):
(defun score (h)
(let* ((hc (histogram h :key (function card-weight))) (his (histogram hc)))
(cond
((and (= 10 (card-number (first h))) (straightp h) (same-color-p h)) ; rf
0)
((and (/= 10 (card-number (first h))) (straightp h) (same-color-p h)) ; sf
(+ 1 (high-card h)))
((= 1 (nth 4 (histogram (histogram h :key (function card-weight))))) ; 4oak
(+ 2 *nnums* (* *nnums* (position 4 hc)) (position 1 hc)))
((and (= 1 (nth 3 his)) (= 1 (nth 2 his))) ; full-house
(+ 3 (* (+ 2 *nnums*) *nnums*) (* *nnums* (position 3 hc)) (position 2 hc)))
((and (same-color-h h) (not (straightp h))) ; flush
(+ 4 (* (+ 3 (* 2 *nnums*)) *nnums*) (score-suite h)))
((straighp h) ; straight
(+ 5 (* (+ 3 (* 2 *nnums*)) *nnums*) (expt *nnums* 5) (score-straight h)))
((and (= 1 (nth 3 his)) (= 2 (nth 1 his))) ; three-of-a-kind
(+ 5 (* (+ 3 (* 2 *nnums*)) *nnums*) (* 2 (expt *nnums* 5))
))
((= 2 (nth 2 (histogram (histogram h :key (function card-weight))))) 7) ; tp
((and (= 1 (nth 2 his)) (= 3 (nth 1 his))) 8) ; op
(t
(+ ... (score-suite h))))))
> (defun player1-winp (hands)
> (let ((hand1 (rank-hand (subseq hands 0 14)))
> (hand2 (rank-hand (subseq hands 15 29))))
Here again these subseq!!!
Just keep a list of hands, each hand being a list of cards.
Then it'll be easier to change the type of the hands when you'll want
to add derived data to each hand into a structure or an object.
Actually, I wouldn't mind if you kept the hands in a sequence and
still used subseq, on the condition that you'd abstract it away.
(let ((rank0 (rank-hand (nth-hand hands 0)))
(rank1 (rank-hand (nth-hand hands 1))))
...)
If you want to write:
(defconstant +hand-size+ 14)
(defun nth-hand (hands index)
(subseq (* +hand-size+ index) (* +hand-size+ (1+ index))))
no problem. As you see in the above LET, when we read the code, we're
not bothered by the details of implementation of nth-hand, and if we
want to change the implementation to have list of lists instead of a
big sequence with sub sequences, we can just changing these
abstractions.
--
__Pascal Bourguignon__ http://www.informatimago.com/
READ THIS BEFORE OPENING PACKAGE: According to certain suggested
versions of the Grand Unified Theory, the primary particles
constituting this product may decay to nothingness within the next
four hundred million years.