##Style
Style
##Documentation
Documentation
##DRY
DRY
##"Magic numbers"
"Magic numbers"
##Naming
Naming
##Functionality
Functionality
##Style
##Documentation
##DRY
##"Magic numbers"
##Naming
##Functionality
Style
Documentation
DRY
"Magic numbers"
Naming
Functionality
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.).