patternpythonMinor
Tic-Tac-Toe class written in Python
Viewed 0 times
writtentoetictacpythonclass
Problem
How well is this Tic Tac Toe code written? Where may I improve it? This is meant to be a platform for a machine learning algorithm.
The code is written so any player may play first. Once a player plays first and starts the game, the code enforces turns. Overwrite protection is also enforced.
```
#! /usr/bin/python3
class xo:
def __init__(self):
self.board=[[0,0,0],[0,0,0],[0,0,0]];
self.sym=[' ','0','X'];
self.turn=0;
self.modResX=-1;
self.modResO=-1;
self.won=False;
def setPos(self,posx,posy,who):
if (who>=0 & who<3):
self.board[posx][posy]=who;
return 0;
def setX(self,posx,posy):
# check if X is playing first.
if self.turn==0:
self.modResX=0;
self.modResO=1;
# check if X is not playing out of turn.
if self.turn%2==self.modResX:
# check if we are overwriting a position
if (self.board[posx][posy]==0):
self.board[posx][posy]=2;
self.turn+=1;
self.win(2);
return 0;
else:
return -2;
else:
return -1;
def setO(self,posx,posy):
# check if O is playing first.
if self.turn==0:
self.modResX=1;
self.modResO=0;
# check if O is not playing out of turn.
if self.turn%2==self.modResO:
# check if we are overwriting a position
if (self.board[posx][posy]==0):
self.board[posx][posy]=1;
self.turn+=1;
self.win(1);
return 0;
else:
return -2;
else:
return -1;
def win(self,who):
win=False;
if self.board[0]==[who, who, who]:
win=True;
if self.board[1]==[who, who, who]:
win=True;
if self.board[2]==[who, who, who]:
win=True;
The code is written so any player may play first. Once a player plays first and starts the game, the code enforces turns. Overwrite protection is also enforced.
```
#! /usr/bin/python3
class xo:
def __init__(self):
self.board=[[0,0,0],[0,0,0],[0,0,0]];
self.sym=[' ','0','X'];
self.turn=0;
self.modResX=-1;
self.modResO=-1;
self.won=False;
def setPos(self,posx,posy,who):
if (who>=0 & who<3):
self.board[posx][posy]=who;
return 0;
def setX(self,posx,posy):
# check if X is playing first.
if self.turn==0:
self.modResX=0;
self.modResO=1;
# check if X is not playing out of turn.
if self.turn%2==self.modResX:
# check if we are overwriting a position
if (self.board[posx][posy]==0):
self.board[posx][posy]=2;
self.turn+=1;
self.win(2);
return 0;
else:
return -2;
else:
return -1;
def setO(self,posx,posy):
# check if O is playing first.
if self.turn==0:
self.modResX=1;
self.modResO=0;
# check if O is not playing out of turn.
if self.turn%2==self.modResO:
# check if we are overwriting a position
if (self.board[posx][posy]==0):
self.board[posx][posy]=1;
self.turn+=1;
self.win(1);
return 0;
else:
return -2;
else:
return -1;
def win(self,who):
win=False;
if self.board[0]==[who, who, who]:
win=True;
if self.board[1]==[who, who, who]:
win=True;
if self.board[2]==[who, who, who]:
win=True;
Solution
Short answer: not terribly well! It's a good start (the class is a sensible idea and
Style
Python has a style guide, which you should read and follow. A few highlights:
Documentation
It's good to have a
DRY
Or "don't repeat yourself". For example, note that
You could also significantly simplify
"Magic numbers"
Like
Then the code becomes clearer, e.g.
Naming
As well as the styling of the names, it's worth thinking about their meaning, too. It's best to avoid contractions (e.g.
Functionality
if __name__ == '__main__': is a good way to run the code), but you have made a couple of mistakes. Here are a few pointers.Style
Python has a style guide, which you should read and follow. A few highlights:
- Naming: classes should be
CamelCaseand function/method/attribute names should belowercase_with_underscores.
- Whitespace: you should put spaces around operators, e.g.
self.board = [...]and after commas (e.g.self.board = [[0, 0, 0], ...]).
- Semicolons: just don't!
Documentation
It's good to have a
README, although it's not clear what format it's in, but much better is to include docstrings for modules, classes, methods and functions, so that the script has documentation built right into it. This can also be used to generate formatted documentation (see e.g. Sphinx) and by your IDE to provide tooltips and even e.g. type hinting.DRY
Or "don't repeat yourself". For example, note that
setX and setO are almost exactly the same; you could refactor this to a single method which is called by the other methods as appropriate.You could also significantly simplify
main by using loops to reduce repetition, e.g.:def main():
print("Hello")
g = xo()
g.showBoard()
now, nxt = g.setX, g.setO
for x, y in [(2, 2), (1, 1), ...]:
print(now(x, y))
g.showBoard()
print(g.won)
now, nxt = nxt, now"Magic numbers"
Like
1 and 2 (for X and O) and -1 and -2 (not clear) are best avoided - they don't provide any useful information to the reader and make refactoring very difficult. Instead, use named "constants", e.g.:PLAYER_O = 1
PLAYER_X = 2Then the code becomes clearer, e.g.
self.win(PLAYER_X) tells you what's actually happening.Naming
As well as the styling of the names, it's worth thinking about their meaning, too. It's best to avoid contractions (e.g.
self.symbols is clearer than self.sym, and I've no idea what e.g. modResX really means). Better names means more readable code, which is a key aim of Python.Functionality
- It's conventional for a method to either change state and
return None, or to return something but not change state -winreturns something and changes state (although the return value is ignored anyway).
- Rather than
showBoard, I would be inclined to implement a__str__method that returns the "friendly" representation of the board; then you can useprint(g)rather thang.showBoard(). If you do retain the current functionality, note that again there's no need for areturnvalue that's never used.
0is the default first argument torange, so you can write simply e.g.for i in range(3):, but note that it's neater to iterate over the board itself (i.e.for row in self.board:, etc.).
Code Snippets
def main():
print("Hello")
g = xo()
g.showBoard()
now, nxt = g.setX, g.setO
for x, y in [(2, 2), (1, 1), ...]:
print(now(x, y))
g.showBoard()
print(g.won)
now, nxt = nxt, nowPLAYER_O = 1
PLAYER_X = 2Context
StackExchange Code Review Q#102570, answer score: 6
Revisions (0)
No revisions yet.