patternpythonMinor
Python3 command line chess
Viewed 0 times
chesscommandpython3line
Problem
I have written command line chess in
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
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
Multiple instances
Count should be an instance method (
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 (
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
Becomes:
Booleans
A Boolean is either true or false, much clearer than 0 or 1, so I suggest using:
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
Is very crippling:
-
Reading (for example)
-
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
Here
Assign booleans directly
Becomes just:
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 movesCount 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 movingBecomes:
self.has_already_moved = 0Booleans
A Boolean is either true or false, much clearer than 0 or 1, so I suggest using:
self.has_already_moved = FalsePretty 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] = selfIs 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 avoidedfor 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
staticmethoddef 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 = 0Becomes just:
check2 = n - J >= 0Code Snippets
class ChessBoard:
count = 0 # This is the counter to keep track of movesdef __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 movingself.has_already_moved = 0self.has_already_moved = FalseContext
StackExchange Code Review Q#116416, answer score: 6
Revisions (0)
No revisions yet.