patternpythonMinor
Tic-Tac-Toe project in Python
Viewed 0 times
toetictacprojectpython
Problem
Here's a Tic-Tac-Toe program I wrote in Python. How can I improve it (.eg. make it more readable)?
```
class Gameboard():
def __init__(self):
self.gameboard = {1:' ' , 2: ' ', 3: ' ', 4:' ', 5:' ', 6:' ', 7:' ', 8:' ', 9:' '}
def setPeice(self, user, position, gameboard):
gameboard[position] = user
return gameboard
@property
def gameBoard(self):
'''
need to learn more about properties. They seem to be used primarily as
getters and setters. ??
'''
return self.gameboard
def clearboard(self):
self.gameboard = {1:' ' , 2: ' ', 3: ' ', 4:' ', 5:' ', 6:' ', 7:' ', 8:' ', 9:' '}
def is_place_taken(self, gameboard, index):
if gameboard[index] != ' ' :
return True
def is_board_full(self, gameboard):
for index in range(1, 10):
if gameboard[index] == ' ':
return False
return True
def is_game_won(self, gameboard):
win_conds = ((1,2,3), (4,5,6), (7,8,9), (1,4,7), (2,5,8), (3,6,9), (1,5,9), (3,5,7))
for win_cond in win_conds:
if gameboard[win_cond[0]] == gameboard[win_cond[1]] and gameboard[win_cond[1]] == gameboard[win_cond[2]] and gameboard[win_cond[0]]!= ' ':
return True
def printBoard(self,gameboard):
index = 0
for row in range(1,4):
for column in range(1,4):
index += 1
if column != 3:
print(gameboard[index], end='')
print('|', end='')
else:
print(gameboard[index])
class Game():
def on_start(self):
'''
Called on initial start and in subsequent restarts
'''
self.controlBoard = Gameboard()
self.gameboard = self.controlBoard.gameBoard
self.playerOne ='o'
self.playerTwo = 'x'
print('Welcome to tic-tac-toe')
print("What is player one's name?
```
class Gameboard():
def __init__(self):
self.gameboard = {1:' ' , 2: ' ', 3: ' ', 4:' ', 5:' ', 6:' ', 7:' ', 8:' ', 9:' '}
def setPeice(self, user, position, gameboard):
gameboard[position] = user
return gameboard
@property
def gameBoard(self):
'''
need to learn more about properties. They seem to be used primarily as
getters and setters. ??
'''
return self.gameboard
def clearboard(self):
self.gameboard = {1:' ' , 2: ' ', 3: ' ', 4:' ', 5:' ', 6:' ', 7:' ', 8:' ', 9:' '}
def is_place_taken(self, gameboard, index):
if gameboard[index] != ' ' :
return True
def is_board_full(self, gameboard):
for index in range(1, 10):
if gameboard[index] == ' ':
return False
return True
def is_game_won(self, gameboard):
win_conds = ((1,2,3), (4,5,6), (7,8,9), (1,4,7), (2,5,8), (3,6,9), (1,5,9), (3,5,7))
for win_cond in win_conds:
if gameboard[win_cond[0]] == gameboard[win_cond[1]] and gameboard[win_cond[1]] == gameboard[win_cond[2]] and gameboard[win_cond[0]]!= ' ':
return True
def printBoard(self,gameboard):
index = 0
for row in range(1,4):
for column in range(1,4):
index += 1
if column != 3:
print(gameboard[index], end='')
print('|', end='')
else:
print(gameboard[index])
class Game():
def on_start(self):
'''
Called on initial start and in subsequent restarts
'''
self.controlBoard = Gameboard()
self.gameboard = self.controlBoard.gameBoard
self.playerOne ='o'
self.playerTwo = 'x'
print('Welcome to tic-tac-toe')
print("What is player one's name?
Solution
Strive for self-documenting code
Python's
Is understandable but not obvious, using
Kind of better, now let's use native iteration:
Now we can define a constant:
That reads as "the board is full if all the squares are not empty", you may prefer "the board is full if not at least one one square is empty"), expressed as:
The most self-documenting of all is probably using a helper:
The code now looks like a high level description of the problem, not a specific implementation.
You pass
Given that all the methods in
Simplify
Do not start by writing a lot of loops and
"To print a board, I print each one of its line"
"To print a line, I print the squares joined by
"To divide a board into lines, I divide it in chunks of 3"
If you are wondering, dividing a list in chunks of x is present on StackOverflow: https://stackoverflow.com/questions/10364391/how-to-split-python-list-into-chunks-of-equal-size
This is much clearer and modular then your original version.
Python's
any and all are extraordinarily useful, but few people know about them, for example:def is_board_full(self, gameboard):
for index in range(1, 10):
if gameboard[index] == ' ':
return False
return TrueIs understandable but not obvious, using
all it becomes:def is_board_full(self, gameboard):
return all(gameboard[index] != ' ' for index in range(1, 10))Kind of better, now let's use native iteration:
def is_board_full(self, gameboard):
return all(square != ' ' for square in gameboard.values())Now we can define a constant:
EMPTY = ' '
...
def is_board_full(self, gameboard):
return all(square != EMPTY for square in gameboard.values())That reads as "the board is full if all the squares are not empty", you may prefer "the board is full if not at least one one square is empty"), expressed as:
def is_board_full(self, gameboard):
return not any(square == EMPTY for square in gameboard.values())The most self-documenting of all is probably using a helper:
def is_there_at_least_one_empty(self, gameboard):
return any(square == EMPTY for square in gameboard.values())
def is_board_full(self, gameboard):
return not self.is_there_at_least_one_empty(gameboard)The code now looks like a high level description of the problem, not a specific implementation.
@staticmethodYou pass
self into your methods, but most do not use it. A method that does not use self is called a static method and the convention is not to pass self to those methods by applying the @staticmethod decorator:@staticmethod
def is_there_at_least_one_empty(gameboard):
return any(square == EMPTY for square in gameboard.values())
@staticmethod
def is_board_full(gameboard):
return not GameBoard.is_there_at_least_one_empty(gameboard)Given that all the methods in
Gameboard are static you may consider using top-level functions, and achieve separation of concerns by putting those in a separate file. Using a class adds complexity and in this case you do not seem to need nor want one (using only static methods defies OOP completely)Simplify
Do not start by writing a lot of loops and
ifs, you start with a description of the problem:"To print a board, I print each one of its line"
"To print a line, I print the squares joined by
|""To divide a board into lines, I divide it in chunks of 3"
If you are wondering, dividing a list in chunks of x is present on StackOverflow: https://stackoverflow.com/questions/10364391/how-to-split-python-list-into-chunks-of-equal-size
def split_into_lines(board):
return zip(*[iter(board.values())]*3)
def _print_line(line):
print(' | '.join(line))
def print_gameboard(board):
for line in split_into_lines(board):
_print_line(line)This is much clearer and modular then your original version.
Code Snippets
def is_board_full(self, gameboard):
for index in range(1, 10):
if gameboard[index] == ' ':
return False
return Truedef is_board_full(self, gameboard):
return all(gameboard[index] != ' ' for index in range(1, 10))def is_board_full(self, gameboard):
return all(square != ' ' for square in gameboard.values())EMPTY = ' '
...
def is_board_full(self, gameboard):
return all(square != EMPTY for square in gameboard.values())def is_board_full(self, gameboard):
return not any(square == EMPTY for square in gameboard.values())Context
StackExchange Code Review Q#122083, answer score: 4
Revisions (0)
No revisions yet.