patternpythonMinor
English Checkers game
Viewed 0 times
gameenglishcheckers
Problem
I've written this English checkers game. Meanwhile, it's a human vs. human game, although it is easily extensible to a game between other players (such as a computer one).
``
# move - a tuple of squares - the first square is the piece we want to
# move, and the others are the sequence of moves. Usually that tuple
# is 2 squares long - it's only longer if the player made a multiple
# jump.
# In some places of the program, the word "edges" will mean pairs of
# squares that have only one reachable square between them (in a
# diagonal line), so you can jump between them if they are both empty
# and there is an opponent's piece in the middle.
# The square in the middle of the two edges will just be called
# "the middle".
SQUARES = [s for s in xrange(1, 36) if s%9 != 0]
# a "jump" means both single and multiple jumps.
class MovingErrors:
"""A namespace for error constants for illegal moves."""
NotASquare = "The move included a number that is not a square."
TooShort = ("The move should start at one square and finish at "
"another one.")
NotYourPiece = "The player should move his own piece."
MoveToPiece = "The player should move to an empty square."
LongSimpleMove = "A simple mov
``
"""Play English Checkers.
see https://en.wikipedia.org/wiki/English_draughts
"""
from collections import namedtuple
from itertools import cycle
# square - a number between 1 and 35, that isn't divisible by 9:
# . 35 . 34 . 33 . 32
# 31 . 30 . 29 . 28 .
# . 26 . 25 . 24 . 23
# 22 . 21 . 20 . 19 .
# . 17 . 16 . 15 . 14
# 13 . 12 . 11 . 10 .
# . 8 . 7 . 6 . 5
# 4 . 3 . 2 . 1 .
# Pay attention that in this representation, the numbers that are
# divisible by 9 are skipped - thus, two squares are adjacents if and
# only if their difference is 4 or 5. Also, this representation may
# be different from the representation that is used in the program's
# interface (for example, in UserPlayer`).# move - a tuple of squares - the first square is the piece we want to
# move, and the others are the sequence of moves. Usually that tuple
# is 2 squares long - it's only longer if the player made a multiple
# jump.
# In some places of the program, the word "edges" will mean pairs of
# squares that have only one reachable square between them (in a
# diagonal line), so you can jump between them if they are both empty
# and there is an opponent's piece in the middle.
# The square in the middle of the two edges will just be called
# "the middle".
SQUARES = [s for s in xrange(1, 36) if s%9 != 0]
# a "jump" means both single and multiple jumps.
class MovingErrors:
"""A namespace for error constants for illegal moves."""
NotASquare = "The move included a number that is not a square."
TooShort = ("The move should start at one square and finish at "
"another one.")
NotYourPiece = "The player should move his own piece."
MoveToPiece = "The player should move to an empty square."
LongSimpleMove = "A simple mov
Solution
I’m just going to dive in and run the script. Let’s see what happens!
-
I get this instruction:
I’m nitpicking, but the correct spelling is “separate”.
-
More concerningly, I’m not sure what I’m supposed to type here. How do I specify a square? Some sample input would have been useful, because I just typed the first thing that came to mind:
Looks like you’ve failed to range-check my input. Be careful – just because I give you something that looks like an integer doesn’t mean it’s correct.
However, brownie points for not falling over when I try to type in non-numeric input.
One case that could do with slight tidying: what if I specify multiple dashes?
That should also count as invalid input.
-
If I peek under the hood, I found a comment which seems to specify numbers that line up with cells. Okay, let me try that.
Hmm, it didn’t behave quite as I expected (actual vs. expected on left/right):
I’m not sure if that’s a bug or a misunderstanding on my part – but it is weird.
I played a few more moves, and they didn’t quite line up with the grid in the comment at the top of the file.
-
This is a rather unfriendly message:
Don’t be condescending to your users, especially if you haven’t provided any good instructions for playing the game. It’s not my fault if I don’t know what I should be doing.
-
What’s going on with the number 9?
The best thing you could do for this program is improve the introductory process. It’s very difficult (perhaps impossible) to work out how to play the game from the instructions printed to stdout. The quality of your code is irrelevant if I can’t play the game.
I don’t have time to review the code properly, but here are a few things that jumped out at me:
-
I was a bit confused by the name of the
-
There isn’t much wrong PEP 8-wise, except a bit of missing whitespace around operators and the fact that functions should be separated by two blank lines, not one.
-
There’s a spelling error in a few method names: “avaliable” should be “available”.
-
The docstring for
If the move has an "stupid error" (explained later), raise
ValueError with that error from MovingErrors. Otherwise, do
nothing.
Stupid error - TooShort, NotASquare, NotYourPiece, MoveToPiece,
NotKing.
I’m assuming the list of errors in the line below wasn’t what you meant by “explained later”, but I don’t find any other reference to stupid errors if I grep for “stupid”.
I would also refrain from using derogatory terms like “stupid” to describe errors.
-
I think this is just a bit bizarre:
If you really want to do an XNOR operator, I think a more intuitive construction is
but the problem isn’t so much the way the condition is written, as the fact that I don’t know why the condition is significant. There isn’t a comment to explain why the code has been written this way.
In general, there aren’t many comments in this file, which make it hard to read, review
-
I get this instruction:
What's your move? Seperate the squares by dashes (-).
I’m nitpicking, but the correct spelling is “separate”.
-
More concerningly, I’m not sure what I’m supposed to type here. How do I specify a square? Some sample input would have been useful, because I just typed the first thing that came to mind:
What's your move? Seperate the squares by dashes (-). 23-45
Traceback (most recent call last):
File "checkers.py", line 417, in
play_display_checkers(UserPlayer, UserPlayer, upper_color=State.WHITE)
File "checkers.py", line 357, in play_display_checkers
display_checkers(checkers(red, white), upper_color)
File "checkers.py", line 347, in display_checkers
for _, state in game:
File "checkers.py", line 332, in checkers
move = player(state, str(err))
File "checkers.py", line 374, in UserPlayer
move = map(human_square_to_normal, human_squares)
File "checkers.py", line 302, in human_square_to_normal
return SQUARES[human_s-1]
IndexError: list index out of range
Looks like you’ve failed to range-check my input. Be careful – just because I give you something that looks like an integer doesn’t mean it’s correct.
However, brownie points for not falling over when I try to type in non-numeric input.
One case that could do with slight tidying: what if I specify multiple dashes?
Invalid input. Try again: 1-3-5
The player should move to an empty square.
That should also count as invalid input.
-
If I peek under the hood, I found a comment which seems to specify numbers that line up with cells. Okay, let me try that.
. 35 . 34 . 33 . 32
31 . 30 . 29 . 28 .
. 26 . 25 . 24 . 23
22 . 21 . 20 . 19 .
. 17 . 16 . 15 . 14
13 . 12 . 11 . 10 .
. 8 . 7 . 6 . 5
4 . 3 . 2 . 1 .
Hmm, it didn’t behave quite as I expected (actual vs. expected on left/right):
What's your move? Seperate the squares by dashes (-). 12-16
y y y y # y y y y
y y y y # y y y y
y y y y # y y y y
. . . . # . . . .
x . . . # . x . .
. x x x # x . x x
x x x x # x x x x
x x x x # x x x x
I’m not sure if that’s a bug or a misunderstanding on my part – but it is weird.
I played a few more moves, and they didn’t quite line up with the grid in the comment at the top of the file.
-
This is a rather unfriendly message:
What's your move? Seperate the squares by dashes (-). 9-15
What. A simple move should move a piece to an adjacent square, and a jump should jump above opponents. Is that hard?
Don’t be condescending to your users, especially if you haven’t provided any good instructions for playing the game. It’s not my fault if I don’t know what I should be doing.
-
What’s going on with the number 9?
The best thing you could do for this program is improve the introductory process. It’s very difficult (perhaps impossible) to work out how to play the game from the instructions printed to stdout. The quality of your code is irrelevant if I can’t play the game.
I don’t have time to review the code properly, but here are a few things that jumped out at me:
-
I was a bit confused by the name of the
is_square() function, because it conflicts with my idea of a square number. Perhaps rename to is_grid_square()?-
There isn’t much wrong PEP 8-wise, except a bit of missing whitespace around operators and the fact that functions should be separated by two blank lines, not one.
-
There’s a spelling error in a few method names: “avaliable” should be “available”.
-
The docstring for
stupid_errors reads:If the move has an "stupid error" (explained later), raise
ValueError with that error from MovingErrors. Otherwise, do
nothing.
Stupid error - TooShort, NotASquare, NotYourPiece, MoveToPiece,
NotKing.
I’m assuming the list of errors in the line below wasn’t what you meant by “explained later”, but I don’t find any other reference to stupid errors if I grep for “stupid”.
I would also refrain from using derogatory terms like “stupid” to describe errors.
-
I think this is just a bit bizarre:
# == is used as an XNOR operator here
if (rank(square) % 2 == 1) == (upper_color == State.WHITE):If you really want to do an XNOR operator, I think a more intuitive construction is
if ~(condition1 ^ condition2)but the problem isn’t so much the way the condition is written, as the fact that I don’t know why the condition is significant. There isn’t a comment to explain why the code has been written this way.
In general, there aren’t many comments in this file, which make it hard to read, review
Code Snippets
# == is used as an XNOR operator here
if (rank(square) % 2 == 1) == (upper_color == State.WHITE):if ~(condition1 ^ condition2)Context
StackExchange Code Review Q#105479, answer score: 4
Revisions (0)
No revisions yet.