3
\$\begingroup\$

Recently I've been developing a chess game in Python - using pygame - just as a project for myself. I have a class for each piece type, and in each class, I have a method called find_available_moves which returns an array representing the board showing where the piece is legally allowed to move. I've accomplished this, but I feel like there is a lot of repetition and I'm unsure how to make my code more concise.

Here is the method from the Rook class for instance:

def find_available_moves(self, grid):
    """
    :param grid: Grid class
    :return list: The spaces where the rook can move to
    """
    available_spaces = [[False] * grid.columns for i in range(grid.rows)]

    # Check spaces to the right
    for x in range(self.grid_position[0] + 1, grid.columns):
        # Position to check
        position = (x, self.grid_position[1])

        # If there is nothing in the space it is available
        if grid.board_layout[position[1]][position[0]] is None:
            available_spaces[position[1]][position[0]] = True

        # If there is a piece of the opposite colour in the space it is available, but further spaces are not
        elif grid.board_layout[position[1]][position[0]]['colour'] != self.colour:
            available_spaces[position[1]][position[0]] = True
            break

        # If there is a piece of the same colour in the space it is not available, neither are further spaces
        else:
            break

    # Check spaces to the left
    for x in range(self.grid_position[0] - 1, -1, -1):
        # Position to check
        position = (x, self.grid_position[1])

        # If there is nothing in the space it is available
        if grid.board_layout[position[1]][position[0]] is None:
            available_spaces[position[1]][position[0]] = True

        # If there is a piece of the opposite colour in the space it is available, but further spaces are not
        elif grid.board_layout[position[1]][position[0]]['colour'] != self.colour:
            available_spaces[position[1]][position[0]] = True
            break

        # If there is a piece of the same colour in the space it is not available, neither are further spaces
        else:
            break

    # Check spaces below
    for y in range(self.grid_position[1] + 1, grid.rows):
        # Position to check
        position = (self.grid_position[0], y)

        # If there is nothing in the space it is available
        if grid.board_layout[position[1]][position[0]] is None:
            available_spaces[position[1]][position[0]] = True

        # If there is a piece of the opposite colour in the space it is available, but further spaces are not
        elif grid.board_layout[position[1]][position[0]]['colour'] != self.colour:
            available_spaces[position[1]][position[0]] = True
            break

        # If there is a piece of the same colour in the space it is not available, neither are further spaces
        else:
            break

    # Check spaces above
    for y in range(self.grid_position[1] - 1, -1, -1):
        # Position to check
        position = (self.grid_position[0], y)

        # If there is nothing in the space it is available
        if grid.board_layout[position[1]][position[0]] is None:
            available_spaces[position[1]][position[0]] = True

        # If there is a piece of the opposite colour in the space it is available, but further spaces are not
        elif grid.board_layout[position[1]][position[0]]['colour'] != self.colour:
            available_spaces[position[1]][position[0]] = True
            break

        # If there is a piece of the same colour in the space it is not available, neither are further spaces
        else:
            break

    return available_spaces

So is there any way that I can make this more simple, by perhaps using another method to test what is in each grid space? I'm just not sure how to go about implementing something like that.

Thank you for your help.

Variable definitions:

  • The grid variable is just a class which contains information on the current layout of the board and is responsible for drawing the pieces in the correct locations.

  • grid.rows and grid.columns are integers containing the dimensions of the board.

  • grid.board_layout is an array containing a dictionary of the form {'piece': 'rook', 'colour': 'white'} where there is a piece and containing None where there isn't.

  • Rook.grid_position is a tuple containing the grid coordinates of the piece.

\$\endgroup\$

closed as off-topic by Dair, Mast, Donald.McLean, pacmaninbw, Grajdeanu Alex. Aug 7 at 5:56

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Dair, Mast, Donald.McLean, pacmaninbw, Grajdeanu Alex.
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Since this is part of a class, we'd need the rest of the class to see whether it makes sense or not. A usage example of the class would be helpful too. \$\endgroup\$ – Mast Aug 6 at 13:11
3
\$\begingroup\$

The difference between the code for moving right:

    for x in range(self.grid_position[0] + 1, grid.columns):
        # Position to check
        position = (x, self.grid_position[1])

        # If there is nothing in the space it is available
        if grid.board_layout[position[1]][position[0]] is None:
            available_spaces[position[1]][position[0]] = True

        # If there is a piece of the opposite colour in the space it is available, but further spaces are not
        elif grid.board_layout[position[1]][position[0]]['colour'] != self.colour:
            available_spaces[position[1]][position[0]] = True
            break

        # If there is a piece of the same colour in the space it is not available, neither are further spaces
        else:
            break

and for moving forward is just the definition of position:

        position = (self.grid_position[0], y)

So you can certainly factor out a function looking something like this:

def update_line(chessman, grid, available_spaces, line_positions):
    for position in line_positions:

        # If there is nothing in the space it is available
        if grid.board_layout[position[1]][position[0]] is None:
            available_spaces[position[1]][position[0]] = True

        # If there is a piece of the opposite colour in the space it is available, but further spaces are not
        elif grid.board_layout[position[1]][position[0]]['colour'] != chessman.colour:
            available_spaces[position[1]][position[0]] = True
            break

        # If there is a piece of the same colour in the space it is not available, neither are further spaces
        else:
            break

This can be shared between the rook, bishop, and queen. The rook would call it something like

    update_line(self, grid, available_spaces,
                ((x, self.grid_position[1]) for x in range(self.grid_position[0] + 1, grid.columns))
    update_line(...)
    ...
\$\endgroup\$

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