HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python3 command line chess

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
chesscommandpython3line

Problem

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', boa

Solution

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

Code Snippets

class ChessBoard:
    count = 0 # This is the counter to keep track of moves
def __str__(self):
    board = self.draw_board(self.objectboard.T)
    return '\n'.join((map(str, reversed(board)))) + '\n'
self.count = 0 # To keep track if it just started moving
self.has_already_moved = 0
self.has_already_moved = False

Context

StackExchange Code Review Q#116416, answer score: 6

Revisions (0)

No revisions yet.