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

Tic-Tac-Toe project in Python

Submitted by: @import:stackexchange-codereview··
0
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?

Solution

Strive for self-documenting code

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 True


Is 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.
@staticmethod

You 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 True
def 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.