I see you've removed the bugs. Good job. Let's clean it up some more.
Zero -vs- Naught
It is a little disconcerting to read the code and see '0' not in board
or if board[idx] == '0'
. It looks like it is hard-coded checks for the "Naughts" player, when it is in fact, a check for an empty square. '0'
and 'O'
are not all that different in certain fonts.
It could be an improvement to change identifier of the empty squares. But what to change it to?
A space comes to mind. It looks empty, just like the square. More over,
print("""
... omitted ...
""".format(*list(x if x != '0' else ' ' for x in board)))
could be reduced to:
print("""
... omitted ...
""".format(*list(x for x in board)))
since the empty indicator would already be a space. And since board
is already a list
, this could further reduce to:
print("""
... omitted ...
""".format(*board))
which is a definite win.
Deconstructing Assignment
What does this statement do, in 7 words or less?
turn, user = 1, 'X' if bool(randint(0, 1)) else 'O'
I can't come up with one 7 word sentence, but perhaps you can. If so, same question for this statement:
board, codes, turn, user = ['0'] * 9, ('c1', 'c2', 'c3', 'b1', 'b2', 'b3',
'a1', 'a2', 'a3'), turn, user = 1, 'X' if bool(randint(0, 1)) else 'O'
It is similar to the previous statement, in that a couple of variables on the left are being assigned values from the right. The problem is none of these variables are related to each other. If you have an XYZ coordinate, using x, y, z = 0, 1, 2
can be reasonable, but for unrelated concepts, use separate statements:
turn = 1
user = 'X' if bool(randint(0, 1)) else 'O'
Cohesive Code
There are 21 lines between code = input().strip().lower()
and print("Hmm, that's not a valid input.")
. Between those lines, we have a check for a win by either player, a check for a draw game, code to switch players, and code to increment the turn number. Because the code for these things is only valid when the move is valid, 4 levels of indenting are needed.
The while True
loop is doing double duty. It is looping once for each turn of the game, where the state of the game changes, and once for each invalid input, where the state of the game doesn't change.
Code is more cohesive when related lines of code near one another. So the code should be organized more like:
while True:
# get and validate move
# update board
# display new board
# check for win or draw game
# switch players
# increment turn number
This also reduces the amount of indentation you need in the code, which is sometimes called "left-leaning" code.
The "get and validate move" will have its own loop. It could be written in place, but it might be better moved to its own function:
def get_player_move(board, user):
print(f'Player {user}: ')
while True:
code = input().strip().lower()
if code in CODES:
idx = CODES.index(code)
if board[idx] == ' ':
return idx
print('Whoops! You need to pick an empty space.')
else:
print("Hmm, that's not a valid input.")
if turn >= 5:
What is this 5
, and where did it come from?
Ok, in playing Naughts and Crosses, you win by getting 3 of your symbol in a row. Since you can only place one symbol per turn, and you take turns with another player, you can't possibly win before turn 5.
But do you really need to check that? The code would work perfectly without that check. The amount of work the check saves is minimal. More over, if you wanted to change this to a 3 player game, or play on a larger grid and get more than 3 in row, etc., the check would need to be changed.
Perhaps better is to remove the check altogether. It doesn't save enough work to call out as a special case.
Draw Game
Again, Naughts and Crosses is a game is played until someone wins, or until no moves are left. With 9 spaces to play in, there can be at most 9 turns. This leads to a different formulation of the "draw game" detection.
Instead of:
while True:
... omitted ...
if check_for_win(board):
print(f'{user} won in {turn} turns! Congratulations!')
break
elif '0' not in board:
print("Aw, it's a draw!")
break
you could use a for ... else
loop:
for _ in range(9):
... omitted ...
if check_for_win(board):
print(f'{user} won in {turn} turns! Congratulations!')
break
else:
print("Aw, it's a draw!")
What is happening here?
If the for
loop execution runs to completion, the else:
clause at the end of the loop is executed. If the for
loop execution is interrupt by a break
, the loop terminates and the else:
clause is not executed. This is pattern is usually used in a search, where you loop over something until you find the desired item, and if you don't find it you do something "else", but it works just as well here, where you play until a win, or there are no more moves.
The 9
isn't quite as magic as the 5
was, but we can still get rid of it. It is the number of moves which can be made, which is the size of the board. At the same time, we can absorb the turn
variable into the for-loop.
for turn in range(1, len(board) + 1):
... omitted ...
if check_for_win(board):
print(f'{user} won in {turn} turns! Congratulations!')
break
else:
print("Aw, it's a draw!")
numpy
, scipy
, and bitwise operators
While you are hoping for optimizations utilizing numpy
, scipy
or bitwise operators, these are the wrong tools for the job.
I held my tongue in my previous review, but your code could be made much simpler without numpy
.
First, consider what does the following condition do?
' ' not in row and len(set(row)) == 1
First, it ensures that the row doesn't contain any empty cells, and then it forms a set
of the contents of the cells, removing duplicates, and if the row
contained only 1 symbol (which is not the empty symbol), we have found a winning row.
That is a mouthful to describe. What is the winning condition for the game?
A row (or column or diagonal) with 3 of the same symbols.
That sounds way simpler. If you pass user
to the check_for_win
function, then the winning condition would be [user, user, user]
found in a row (or column, or diagonal).
winning_pattern = [user] * 3
...
if row == winning_pattern:
return True
...
No expensive set()
construction. No weird testing the length of the set.
And now to get rid of numpy
. With 3 rows, 3 columns, and 2 diagonals, all we need are 8 slices of the board. And a slice
is a first class object in Python:
ROWS = (slice(0, 3, 1), slice(3, 6, 1), slice(6, 9, 1),
slice(0, 9, 3), slice(1, 10, 3), slice(2, 11, 3),
slice(0, 12, 4), slice(3, 9, 2))
def check_for_win(board, user):
winning_pattern = [user] * 3
return any(board[row] == winning_pattern for row in ROWS)
Refactored Code
from random import randint
CODES = ('c1', 'c2', 'c3', 'b1', 'b2', 'b3', 'a1', 'a2', 'a3')
ROWS = (slice(0, 3, 1), slice(3, 6, 1), slice(6, 9, 1),
slice(0, 9, 3), slice(1, 10, 3), slice(2, 11, 3),
slice(0, 12, 4), slice(3, 9, 2))
def print_board(board):
print("""
+---+---+---+
c | {} {} {} |
+ + + +
b | {} {} {} |
+ + + +
a | {} {} {} |
+---+---+---+
1 2 3
""".format(*board))
def check_for_win(board, user):
winning_pattern = [user] * 3
return any(board[row] == winning_pattern for row in ROWS)
def get_player_move(board, user):
print(f'Player {user}: ')
while True:
code = input().strip().lower()
if code in CODES:
idx = CODES.index(code)
if board[idx] == ' ':
return idx
print('Whoops! You need to pick an empty space.')
else:
print("Hmm, that's not a valid input.")
def tic_tac_toe():
board = [' '] * 9
user = 'X' if bool(randint(0, 1)) else 'O'
print(f'Welcome! First to play is {user}!')
print_board(board)
for turn in range(1, len(board) + 1):
idx = get_player_move(board, user)
board[idx] = user
print_board(board)
if check_for_win(board, user):
print(f'{user} won in {turn} turns! Congratulations!')
break
user = 'X' if user == 'O' else 'O'
else:
print("Aw, it's a draw!")
if __name__ == '__main__':
tic_tac_toe()
Despite adding 2 more functions, the code is 9 lines shorter (ignoring blank lines).