I'm writing a small checkers game in python (international rules) and I need some pointers here and there. For the moment, I am only drawing the board and checking the syntax of moves with a regex.
For review:
- Usage of a value package for passing around game state (grid, turn_count, and current player), is there a better way, or is this acceptable?
- I have some issue with
GRID_HEIGHT
andGRID_WIDTH
. As they are constants and supposed to be reflecting the size of my grid, I know it is bad to let them hardcoded to 10 that way. However, as far as I can tell, I can't set their values after generating the grid. I guess I should compute them and put them in the value package. - As for
transform_response_into_tuples(response)
, I would like to find a neater way to make the tuples (I should say the pairs). I mean it's working but I think it is ugly. - Also, the printing is quite rigid, note the use of empty print statement in order to line break.
- Finally, is there some statement that could be made more "pythonic"?
Here is how looks:
#CONSTANTS SECTION
CLEAR = lambda: os.system('cls' if os.name == 'nt' else 'clear')
GRID_HEIGHT = 10
GRID_WIDTH = 10
#White pawn
WP = "▓"
#White queen
WQ = '£'
#Empty cell
EC = ' '
#Black pawn
BP = '░'
#Black Queen
BQ = '$'
#players
PLAYERS = Enum("Players", "White Black")
#END OF CONSTANTS SECTION
def init_grid():
"""Initialize the new game grid"""
grid = [[EC, BP, EC, BP, EC, BP, EC, BP, EC, BP],
[BP, EC, BP, EC, BP, EC, BP, EC, BP, EC],
[EC, BP, EC, BP, EC, BP, EC, BP, EC, BP],
[BP, EC, BP, EC, BP, EC, BP, EC, BP, EC],
[EC, EC, EC, EC, EC, EC, EC, EC, EC, EC],
[EC, EC, EC, EC, EC, EC, EC, EC, EC, EC],
[EC, WP, EC, WP, EC, WP, EC, WP, EC, WP],
[WP, EC, WP, EC, WP, EC, WP, EC, WP, EC],
[EC, WP, EC, WP, EC, WP, EC, WP, EC, WP],
[WP, EC, WP, EC, WP, EC, WP, EC, WP, EC]]
return grid
def move(value_package):
"""This function moves the pieces according to the player's wish"""
print("Turn : ", value_package["turn_count"])
if value_package["cur_turn"] == PLAYERS.White:
print("White's turn :\n")
print_board(value_package["board"])
#Ask for command until the syntax is correct
while True:
print("Enter movement :", end="")
if interpret_response(value_package["board"], input()) == True:
value_package["cur_turn"] = PLAYERS.Black
value_package["turn_count"] += 1
break
else:
print("Black's turn :\n")
#print_board(value_package["board"])
#DO THE AI ACTION
value_package["cur_turn"] = PLAYERS.White
#value_package["turn_count"] += 1
def interpret_response(board, response):
"""This functon interprets the response"""
if check_response_syntax(response):
tuples = transform_response_into_tuples(response)
if check_move_legality(board, tuples):
return True
else:
print("Syntax Error")
return False
def check_response_syntax(response):
"""This function checks if the syntax of the response is correct"""
#Example of correct syntax: 7A6B OR 9F9F (Which is not a legal move !)
return False if re.match("^([0-9][A-J]){2}$", response) == None else True
def transform_response_into_tuples(response):
"""This function decompose the response into tuples"""
match = re.findall("([0-9][A-J]){1}", response)
# -65 because ASCII A-Z to integers (remember A == 0 ...)
# -48 because ASCII 1-9 to integers (remember 0 == 0 ...)
l_val1 = ord(match[0][0]) - 48
r_val1 = ord(match[0][1]) - 65
l_val2 = ord(match[1][0]) - 48
r_val2 = ord(match[1][1]) - 65
return ((l_val1, r_val1), (l_val2, r_val2))
def check_move_legality(board, tuples):
"""This function checks if the move is legal"""
return True
def print_board(board):
"""This function is drawing the board"""
print(" A B C D E F G H I J\n")
for i in range(GRID_HEIGHT):
print(i, " |", end="")
for j in range(GRID_WIDTH):
current_cell = board[i][j]
print(current_cell + "|", end="")
print("")
print("")
def main():
""" Entry point """
CLEAR()
print("PY-CHECKERS")
value_package = dict([("board", init_grid()), ("turn_count", 1), ("cur_turn", PLAYERS.White)])
while True:
move(value_package)
-
\$\begingroup\$ Welcome to Code Review! Good job on your first question. \$\endgroup\$SirPython– SirPython2016年02月08日 22:52:35 +00:00Commented Feb 8, 2016 at 22:52
1 Answer 1
The
def check_move_legality(board, tuples): """This function checks if the move is legal""" return True
puts the code on the verge of being off-topic.
The
from
andto
fields is not enough to uniquely identify the move: there could be two multiple captures following different paths.In logical context
None
evaluates toFalse
. You mayreturn re.match("^([0-9][A-J]){2}$", response)
directly. In fact,
return False if condition else True
is considered an anti-pattern.Also I recommend to parse the response directly: syntax validation is a side effect of successful parsing.
Docstrings are not very informative.
It doesn't tell me much that
This function decompose the response into tuples
. Consider something along the lines ofParse response into start and end fields
. If the function has a side effect (such as ultimate change of a board state) it must be in docstring as well.
-
\$\begingroup\$ I agree with you on most of the points. However, I don't really see why parsing the response directly is better than checking it before, then parsing only if the string is right. Sticking with the actual code, I would need to add new checks when parsing. eg :
if len(match) != 2: return False
So it may be better for speed to do it this way (I reckon regexes are pretty slow), but I think it impairs readability. Anyway, fot the current code, this implies that nobody should ever give a syntacticly incorrect string totransform_response_into_tuples
which may be wrong too. \$\endgroup\$Hello_world– Hello_world2016年02月09日 08:30:05 +00:00Commented Feb 9, 2016 at 8:30 -
\$\begingroup\$ "puts the code on the verge of being off-topic." The original question explained what parts of the code had been done, so this isn't really in the realm of nonworking code. \$\endgroup\$SuperBiasedMan– SuperBiasedMan2016年02月09日 12:19:54 +00:00Commented Feb 9, 2016 at 12:19