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;
if ([self.board[0][0], self.board[1][1], self.board[2][2]]==[who,who,who]):
win=True;
transList=list(map(list, zip(*self.board)))
if transList[0]==[who, who, who]:
win=True;
if transList[1]==[who, who, who]:
win=True;
if transList[2]==[who, who, who]:
win=True;
if ([transList[0][0], transList[1][1], transList[2][2]]==[who,who,who]):
win=True;
self.won=win;
return win;
def showBoard(self):
for i in range(0, 3):
for j in range (0,3):
if j<2:
print (self.sym[self.board[i][j]],'| ',end="",flush=True)
else:
print (self.sym[self.board[i][j]],end="",flush=True)
print(end="\n")
print(end="\n")
return 0;
def main():
print("Hello");
g=xo();
g.showBoard();
print(g.setX(2,2));
g.showBoard();
print(g.won);
print(g.setO(1,1));
g.showBoard();
print(g.won);
print(g.setX(0,1));
g.showBoard();
print(g.won);
print(g.setO(1,0));
g.showBoard();
print(g.won);
print(g.setX(1,2));
g.showBoard();
print(g.won);
print(g.setO(2,0));
g.showBoard();
print(g.won);
# Playing out of turn
print(g.setO(0,2));
g.showBoard();
print(g.won);
# Overwriting a position
print(g.setX(2,0));
g.showBoard();
print(g.won);
print(g.setX(0,2));
g.showBoard();
print(g.won);
if __name__ == '__main__':
main()
Readme
XO
A Python class for a game of X and O.
To use the class:
import xo game=xo.xo();
game.showBoard()
This displays:
| X | 0 0 | 0 | X 0 | | X
game.setX(<posx>,<posy>)
This sets 'X' at a certain position. If successful, returns 0; else -1
game.setO(<posx>,<posy>)
This sets 'O' at a certain position. If successful, returns 0; else -1
game.won
After a move, this variable will be
True
if the last player who played has won. Else, it isFalse
.
Updated Version
Followup question
1 Answer 1
Short answer: not terribly well! It's a good start (the class is a sensible idea and 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
CamelCase
and 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 = 2
Then 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 -win
returns 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 areturn
value that's never used. 0
is 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.).
-
\$\begingroup\$ Thank you for the detailed comments. I am fixing the code. I'll post it once it is done. About "DRY" - My thoughts were that for a small function, calling another function involves pushing context to stack and popping it back on return may cause degradation of performance. So, I chose to rewrite most of it. Is there a flaw in my reasoning? I would however like to clean up
win
. \$\endgroup\$Lord Loh.– Lord Loh.2015年09月02日 17:50:33 +00:00Commented Sep 2, 2015 at 17:50 -
1\$\begingroup\$ It might be slightly less performant, but if that's a big issue you're probably using the wrong language to start with! It's unlikely to be a bottleneck, anyway. \$\endgroup\$jonrsharpe– jonrsharpe2015年09月02日 17:52:11 +00:00Commented Sep 2, 2015 at 17:52
-
\$\begingroup\$ pastebin.com/ewkVJDW6 is an updated version. \$\endgroup\$Lord Loh.– Lord Loh.2015年09月10日 19:09:07 +00:00Commented Sep 10, 2015 at 19:09
-
\$\begingroup\$ @LordLoh. If you'd like a follow-up review, see meta.codereview.stackexchange.com/q/1065/32391 \$\endgroup\$jonrsharpe– jonrsharpe2015年09月12日 08:05:27 +00:00Commented Sep 12, 2015 at 8:05
-
\$\begingroup\$ Thank you. I have posted a follow up question - codereview.stackexchange.com/questions/104946/… \$\endgroup\$Lord Loh.– Lord Loh.2015年09月17日 18:35:19 +00:00Commented Sep 17, 2015 at 18:35