4
\$\begingroup\$

I have written command line chess in Python 3.4.3 and the code is included here. The chessboard is printed to standard output as a 2-D array; for now I only have CLI and the output in the command line is as below:

['Rb' '0 ' 'Bb' 'Qb' 'Kb' 'Bb' 'Nb' 'Rb']
['Pb' 'Pb' 'Pb' 'Pb' '0 ' 'Pb' 'Pb' 'Pb']
['0 ' '0 ' 'Nb' '0 ' '0 ' '0 ' '0 ' '0 ']
['0 ' '0 ' '0 ' '0 ' 'Pb' '0 ' '0 ' '0 ']
['0 ' '0 ' '0 ' '0 ' 'Pw' '0 ' '0 ' '0 ']
['0 ' '0 ' '0 ' '0 ' '0 ' 'Qw' '0 ' '0 ']
['Pw' 'Pw' 'Pw' 'Pw' '0 ' 'Pw' 'Pw' 'Pw']
['Rw' 'Nw' 'Bw' '0 ' 'Kw' 'Bw' 'Nw' 'Rw']

Where R:Rook, N:Knight, B:Bishop, Q:Queen, K:King, P:Pawn and the lower case letters denote the corresponding color.

Furthermore, I used exceptions for impossible moves. How can I improve the code so that the program will not terminate when a impossible move is attempted? I have yet to implement castling, check and enpassant. There also needs to be a check for checkmate. I would be glad if you can provide some insight on my code.

The latest code is here.

import numpy as np
import tkinter as tk
# PATH OBSTRUCTIONS, CHECK, PROMOTION AND CASTLING

class ChessBoard:
    count = 0 # This is the counter to keep track of moves
    def __init__(self):
        self.objectboard = self.form_board()
    def __str__(self): 
        string = ''
        board = self.draw_board(self.objectboard.T) # Transpose is necessary to make the board as we are accustomed to
        for i in reversed(range(8)): # It is reversed so that white is at the bottom
            string += str(board[i])  + '\n'
        return string
    def form_board(self): # Forms the board and puts the pieces on the respective positions
        board = np.zeros((8,8), dtype = object)
        # Now we should put the pieces on the board
        WhiteRook1 = Rook(0, 0, 'w', board)
        WhiteRook2 = Rook(7, 0, 'w', board)
        WhiteKnight1 = Knight(1, 0, 'w', board)
        WhiteKnight2 = Knight(6, 0, 'w', board)
        WhiteBishop1 = Bishop(2, 0, 'w', board)
        WhiteBishop2 = Bishop(5, 0, 'w', board)
        WhiteQueen = Queen(3, 0, 'w', board)
        WhiteKing = King(4, 0, 'w', board)
        # Now we should put the pawns
        for i in range(8):
            exec("WPawn" + str(i+1)  + "= Pawn(i, 1, 'w', board)")  
        # This syntax is for changing variable names
        # Now put the black pieces 
        BlackRook1 = Rook(0, 7, 'b', board)
        BlackRook2 = Rook(7, 7, 'b', board)
        BlackKnight1 = Knight(1, 7, 'b', board)
        BlackKnight2 = Knight(6, 7, 'b', board)
        BlackBishop1 = Bishop(2, 7, 'b', board)
        BlackBishop2 = Bishop(5, 7, 'b', board)
        BlackQueen = Queen(3, 7, 'b', board)
        BlackKing = King(4, 7, 'b', board)
        # Now we should put the pawns
        for i in range(8):
            exec("BPawn" + str(i+1)  + "= Pawn(i, 6, 'b', board)")  
        return board 

    def draw_board(self, board):
        func_sym_col = np.vectorize(self.retrieve_piece)
        symbolic_board = func_sym_col(board)
        return symbolic_board

    def retrieve_piece(self, piece):
        if isinstance(piece, ChessPiece):
            return str(piece.symbol+piece.color)
        else:
            return '0 '

    def rules(self, piece, i, j, m, n):
        board = self.objectboard
        #symboard = self.draw_board(board)
        if ((self.__class__.count % 2) == 0):
            if (piece.color == 'b'):
                raise Exception('It is Whites turn to play')
        else:
            if (piece.color == 'w'):
                raise Exception('It is Blacks turn to play')
        piece_type = piece.symbol # Rules depend on the piece
        # Implement check
        check_new_pos = 0 # We should modify this write a loop over other pieces
        opponent_king = 0
        auxboard = []
        if ((m - i) >= 0):
            check1 = 1
        else:
            check1 = 0
        if ((n - j) >= 0):
            check2 = 1
        else:
            check2 = 0

        if piece_type == 'K':
            if (abs(i - m) > 1):
                raise Exception('This is not a valid move for the King')
            elif (abs(j - n) > 1) :
                raise Exception('This is not a valid move for the King')
            elif check_new_pos:
                raise Exception('The King cannot move to a threatened square!!!')
            elif opponent_king:
                raise Exception('You cannot go too close to the opponent king')

        elif piece_type == 'Q':
            if not ((abs((i - m) / (j - n)) == 1) or ((i - m) == 0) or ((j - n) == 0)):
                raise Exception('The queen cannot move like this')
            if (i - m) == 0:
                if check2:
                    auxboard = board[i][j+1:n]
                else:
                    auxboard = board[i][n+1:j]
            elif (j - n) == 0:
                if check1:
                    auxboard = board[i+1:m][j]
                else:
                    auxboard = board[m+1:i][j]
            else:
                if check1 and check2:
                    for ct in range(m - i - 1):
                        auxboard.append(board[i + 1 + ct][j + 1 + ct])
                elif check1 and (not check2):
                    for ct in range(m - i  - 1):
                        auxboard.append(board[i + 1 + ct][j + 1 - ct])
                elif (not check1) and check2:
                    for ct in range(i - m - 1):
                        auxboard.append(board[i + 1 - ct][j +1 + ct])
                elif (not check1) and (not check2):
                    for ct in range(i - m - 1):
                        auxboard.append(board[i + 1 - ct][j + 1 - ct])
            if not (all(p == 0 for p in auxboard)):
                raise Exception('The path is obscured')

        elif piece_type == 'R':
            if not (((i - m) == 0) or ((j - n) == 0)):
                raise Exception('The rook cannot move like this')
            if (i - m) == 0:
                if check2:
                    auxboard = board[i][j+1:n]
                else:
                    auxboard = board[i][n+1:j]
            elif (j - n) == 0:
                if check1:
                    auxboard = board[i+1:m][j]
                else:
                    auxboard = board[m+1:i][j]
            if not (all(p == 0 for p in auxboard)):
                raise Exception('The path is obscured')

        elif piece_type == 'B':
            if not (abs((i - m) / (j - n)) == 1):
                raise Exception('The bishop cannot move like this')
            if check1 and check2:
                for ct in range(m - i - 1):
                    auxboard.append(board[i + 1 + ct][j + 1 + ct])
            elif check1 and (not check2):
                for ct in range(m - i  - 1):
                    auxboard.append(board[i + 1 + ct][j + 1 - ct])
            elif (not check1) and check2:
                for ct in range(i - m - 1):
                    auxboard.append(board[i + 1 - ct][j +1 + ct])
            elif (not check1) and (not check2):
                for ct in range(i - m - 1):
                    auxboard.append(board[i + 1 - ct][j + 1 - ct])
                    print(board[i + 1 - ct][j + 1 - ct])
            if not (all(p == 0 for p in auxboard)):
                raise Exception('The path is obscured')
        elif piece_type == 'N': # The path may be obscured this time
            if not (((abs(i - m) == 2) and (abs(j - n) == 1)) or  ((abs(i - m) == 1) and (abs(j - n) == 2))):
                raise Exception('The knight cannot move like this')

        elif piece_type == 'P':
            if piece.color == 'w':
                if piece.count == 0:
                    if not(((n - j) == 2) or ((n - j) == 1) and ((i - m) == 0)):
                        raise Exception('The pawn cannot move like this')
                elif piece.count != 0:
                    if not((n - j) == 1):
                        raise Exception('The pawn cannot move like this')
            else:
                if piece.count == 0:
                    if not(((n - j) == -2) or ((n - j) == -1) and ((i - m) == 0)):
                        raise Exception('The pawn cannot move like this')
                elif piece.count != 0:
                    if not((n - j) == -1):
                        raise Exception('The pawn cannot move like this')

        # Implement one cannot move to a square containing same color piece
        if board[m][n] != 0: # There is a piece in the final position
            if board[i][j].color == board[m][n].color:# Two pieces are of the same color
                raise Exception("You cannot go to your own pieces location")
            elif board[m][n].symbol == 'K':# The opponent king is in the location
                raise Exception("You cannot eat the KING")
        if ((piece_type == 'P') or (piece_type == 'K')):
            piece.count += 1
        return 1 

    def move(self, position):
        # These two strings are for board coordinates
        letstr = 'abcdefgh'
        numstr = '12345678'
        board = self.objectboard
        if not (len(position) == 4):
            raise ValueError('The position string should consist of 4 characters');
        # Get the final and initial positions
        initial_pos = position[:2]
        final_pos = position[-2:]
        # First perform the checks
        if not (str == type(initial_pos) and (str == type(final_pos))):     # Check if the arguments are strings
            raise TypeError('The supplied positions should be strings!')
        elif not ((initial_pos[0] in letstr) and (initial_pos[1] in numstr)): # Check if they fulfill the condition to be on the board
            raise ValueError('The initial position values should be between a1 and h8')
        elif not ((final_pos[0] in letstr) and (final_pos[1] in numstr)): # Check if they fulfill the condition to be on the board
            raise ValueError('The final position values should be between a1 and h8')
        elif initial_pos == final_pos:
            raise ValueError('Final position should be different from the initial position')
        # Now determine if there is a piece on the initial square 
        i = letstr.index(initial_pos[0]) ; j = numstr.index(initial_pos[1]) # Numerical initial position
        m = letstr.index(final_pos[0]); n = numstr.index(final_pos[1]) # Numerical final position
        if not (isinstance(board[i][j], ChessPiece)):
            raise Exception('There is no chess piece here')
        piece = board[i][j]
        if self.rules(piece, i, j, m, n) != 1:
            raise('This move is not allowed')
        # Move the piece on the chessboard
        piece.movepiece(i, j, m, n, board)
        self.__class__.count += 1 # Increment the counter after each allowed move

class ChessPiece: # This is the base class, all the other specific pieces inherit this class.
    def __init__(self, x, y, color): 
        if not ((int == type(x)) and (int == type(y))):
            raise TypeError(' x and y should be integers!!')
        elif not ((x in range(8)) and (y in range(8))):
            raise ValueError('x and y positions should be between 0 and 7 inclusive')
        elif not ((str == type(color)) and (color in 'wb')):
            raise ValueError('Color should be "w" or "b"')
        self.pos_x = x
        self.pos_y = y
        self.color = color
        # IMPLEMENT PROMOTION HERE
    def movepiece(self, i, j, m, n, chessboard):
        self.pos_x = i
        self.pos_y = j
        chessboard[i][j] = 0 # Set the previous position to be zero
        chessboard[m][n] = self

class King(ChessPiece):
        def __init__(self, x, y, color, chessboard):
            ChessPiece.__init__(self, x, y, color)
            self.symbol = 'K'
            self.count = 0
            chessboard[self.pos_x][self.pos_y] = self

class Queen(ChessPiece):
        def __init__(self, x, y, color, chessboard):
            ChessPiece.__init__(self, x, y, color)
            self.symbol = 'Q'
            chessboard[self.pos_x][self.pos_y] = self

class Rook(ChessPiece):
        def __init__(self, x, y, color, chessboard):
            ChessPiece.__init__(self, x, y, color)  
            self.symbol = 'R'
            chessboard[self.pos_x][self.pos_y] = self

class Bishop(ChessPiece):
        def __init__(self, x, y, color, chessboard):
            ChessPiece.__init__(self, x, y, color)
            self.symbol = 'B'
            chessboard[self.pos_x][self.pos_y] = self

class Knight(ChessPiece):
        def __init__(self, x, y, color, chessboard):
            ChessPiece.__init__(self, x, y, color)
            self.symbol = 'N'
            chessboard[self.pos_x][self.pos_y] = self

class Pawn(ChessPiece):
        def __init__(self, x, y, color, chessboard):
            ChessPiece.__init__(self, x, y, color)
            self.symbol = 'P'
            self.count = 0 # To keep track if it just started moving
            chessboard[self.pos_x][self.pos_y] = self

# These are auxiliary functions
####################################################################
# ACTUAL CODE STARTS HERE
chessboard = ChessBoard()
print(chessboard)
chessboard.move('e2e4')
print(chessboard)
chessboard.move('e7e5')
print(chessboard)
chessboard.move('d1f3')
print(chessboard)
chessboard.move('b8c6')
print(chessboard)
chessboard.move('c1e3')
print(chessboard)
chessboard.move('f8c5')
print(chessboard)
chessboard.move('f3f7')
# Already mate here
print(chessboard)
chessboard.move('h8h5')
print(chessboard)
chessboard.move('h1h4')
print(chessboard)
\$\endgroup\$
  • 2
    \$\begingroup\$ Telling you how to add new functionality is out of scope for CR; do you want a review of what you've currently implemented? \$\endgroup\$ – jonrsharpe Jan 11 '16 at 13:27
  • 2
    \$\begingroup\$ @jonrsharpe Okay I will implement the GUI myself. Apart from that I can have some feedback. \$\endgroup\$ – Vesnog Jan 11 '16 at 13:35
  • \$\begingroup\$ That's fine, but please edit the question accordingly. \$\endgroup\$ – jonrsharpe Jan 11 '16 at 13:53
6
\$\begingroup\$

OO design points

Splitting responsibilities

Why does a Board know how each piece moves? And each piece knows nothing? It makes much more sense to implement a move(self, target_position) for each piece.

Multiple instances

class ChessBoard:
    count = 0 # This is the counter to keep track of moves

Count should be an instance method (self.count) as it currently stands, you can only have one board at a time.

Use built-ins

Writing in Python allows you to avoid explicit indexing and looping, easier code that does the same can be written using built-ins (enumerate, reversed, join ...)

def __str__(self):
    board = self.draw_board(self.objectboard.T)
    return '\n'.join((map(str, reversed(board)))) + '\n'

Names

This has been said hundreds of times in this site, I will repeat it: long descriptive names are a must for good code.

A good rule of thumb is that no name should require a comment next to it to explain it. Just use a more descriptive name in the first place, for example (in the Pawn class):

self.count = 0 # To keep track if it just started moving

Becomes:

self.has_already_moved = 0

Booleans

A Boolean is either true or false, much clearer than 0 or 1, so I suggest using:

self.has_already_moved = False

Pretty printing

Your output format is not nice, it is confusing and user un-friendly, I suggest taking full advantage of Python-3 full Unicode compatibility using the Chess Unicode characters and removing the square brackets and quotations marks.

Avoid arbitrary outside world modifications

Inside each Piece class:

chessboard[self.pos_x][self.pos_y] = self

Is very crippling:

  • Reading (for example) WhiteRook1 = Rook(0, 0, 'w', board) gives me no clue that the board is being modified

  • WhiteRook1 = Rook(0, 0, 'w', board) will fail if the board has any other name than chessboard, a very fragile way of programming.

eval is best avoided

    for i in range(8):
        exec("WPawn" + str(i+1)  + "= Pawn(i, 1, 'w', board)")  

No.

Use a list to store the pieces, not a variable for each piece, this coupled with the pieces knowing out to move will simplify your code.

Declare static methods staticmethod

def retrieve_piece(self, piece):
    if isinstance(piece, ChessPiece):
        return str(piece.symbol+piece.color)
    else:
        return '0 '

Here retrieve_piece is static in that it does not access the internals of a class, so make this clear and declare it as:

@staticmethod
def retrieve_piece(piece):
    if isinstance(piece, ChessPiece):
        return str(piece.symbol+piece.color)
    else:
        return '0 '

Assign booleans directly

    if ((n - j) >= 0):
        check2 = 1
    else:
        check2 = 0

Becomes just:

check2 = n - J >= 0
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks I will look into your comments, much appreciated. \$\endgroup\$ – Vesnog Jan 11 '16 at 15:28
  • \$\begingroup\$ By the way how should I handle illegal moves do you think? \$\endgroup\$ – Vesnog Jan 11 '16 at 16:17
  • \$\begingroup\$ I have also added Tkinter to visualize the board, how can I have the board displayed while I can still execute commands from the command line? What I would like to see is the real time movement of chess pieces, I am attaching the latest version of the code here and I will look in to the other points you have brought up. \$\endgroup\$ – Vesnog Jan 11 '16 at 16:27
2
\$\begingroup\$

To your final question about proper control flow, you can use exceptions for exceptional circumstances. I think it's perfectly acceptable to use them for invalid moves/states. The general pattern you'll want to use is in your CLI input loop:

while True:
    move = # prompt user for a move
    try:
        game.make_move(move)
    except ChessException as e:
        print(e)

So on every turn, the CLI will accept a move from the user and attempt it. If the move is invalid, an exception will be thrown and and input loop will catch this exception, display it, and prompt the user for a move again.

One good practice to take away from this is to not throw generic exceptions (ie. raise Exception('King cannot move to a threatened square')). The reason for this is that higher in the control flow (that is, in the function that calls the function which raises the exception, or perhaps that function that calls the first function) there is no way to differentiate between exceptions sanely. You could compare the message (str(e)), but this is a bad solution (changing the an error string by a character requires you to modify it everywhere else).

Instead, create a base class for all of your chess-related exceptions (I called it ChessException in the above example), and extend that with relevant exceptions. Your inheritance tree could look something like this:

 + ChessException
  \
   + InvalidMoveException
   |\
   | + OccupiedFriendlySquareException
   | + KingThreatenedSquareException
   | + ObscuredPathException
   + InvalidTurnException

There many advantages to using this best practice:

  • You don't just have a catch 'em all try/catch in the input loop, which could consume Python exceptions (like IndexError or NameError), making code hard to debug
  • You can now catch specific exceptions (or specific types like a InvlaidMoveException) and handle them differently than other exceptions (this may not be extremely useful for your CLI use case, but consider if others used your chess module as an API)
  • Your code is now more future proof (want to change the message output by a ObscuredPathException? No problem)
\$\endgroup\$
  • \$\begingroup\$ thanks for the comments, how about updating the tkinter window real time? \$\endgroup\$ – Vesnog Jan 11 '16 at 22:40

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.