7
\$\begingroup\$

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 and GRID_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:

How it 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)
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first question. \$\endgroup\$ – SirPython Feb 8 '16 at 22:52
6
\$\begingroup\$
  • 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 and to fields is not enough to uniquely identify the move: there could be two multiple captures following different paths.

  • In logical context None evaluates to False. You may

        return 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 of Parse 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.

\$\endgroup\$
  • \$\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 to transform_response_into_tuples which may be wrong too. \$\endgroup\$ – Hello_world Feb 9 '16 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 Feb 9 '16 at 12:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.