PyLint is a useful tool for checking your code, ensuring good coding habits. For example, PyLint doesn’t like a lack of spaces around operators or after commas. Neither do my old weak eyes. Instead of this:
pos_matrix=[[' ',' ',' '],[' ',' ',' '],[' ',' ',' ']]
... code like this:
pos_matrix = [[' ', ' ', ' '], [' ', ' ', ' '], [' ', ' ', ' ']]
An if
statement that does a return
or exit
doesn’t need to be followed by an else
or elif
; if the if
statement is true, the execution path doesn’t need to skip around the else
clause, because execution has already left the function.
For clarity:
if pos_matrix[pos[0]-1][pos[1]-1]!=' ':
print('Invalid input')
exit
else:
pos_matrix[pos[0]-1][pos[1]-1]=symbol
for i in range(0,3):
# print rows of game grid
After the if-else
statement is the code to print out the game grid. It appears this code belongs to both paths through the if-else
, where as it will only execute if the False
path is taken. As such, it should be indented to belong as part of the else:
clause:
if pos_matrix[pos[0] - 1][pos[1] - 1] != ' ':
print('Invalid input')
exit
else:
pos_matrix[pos[0] - 1][pos[1] - 1] = symbol
for i in range(0,3):
# print rows of game grid
Alternately, the else:
can be omitted entirely, and the statement in the else:
clause outdented to the same level as the if
statement:
if pos_matrix[pos[0]-1][pos[1]-1]!=' ':
print('Invalid input')
exit
pos_matrix[pos[0]-1][pos[1]-1]=symbol
for i in range(0,3):
# print rows of game grid
Invalid input should not unconditionally terminate the program.
if pos_matrix[pos[0]-1][pos[1]-1]!=' ':
print('Invalid input')
exit
This code makes it harder to use in a larger program, since an invalid input will terminate the entire program.
Instead, a ValueError
exception should be raised:
if pos_matrix[pos[0] - 1][pos[1] - 1] != ' ':
raise ValueError('Invalid input')
This allows the caller to use a try ... except:
structure. The user could enter an invalid input, the function detects this and raises the error. The caller catches the error, and request the user re-enter a valid move.
Without the appropriate try ... except:
statement, raising the exception will also terminate the program.
You decode pos
using [pos[0]-1][pos[1]-1]
multiple times. This is ugly and hard to follow. Use variable names to avoid repeating the same calculations:
row, col = pos[0] - 1, pos[1] - 1
Then you can write:
if pos_matrix[row][col] != ' ':
Instead of looping over range(0, 3)
or [0, 1, 2]
, just use range(3)
:
for i in range(3):
....
def my_tictactoe(pos,symbol):
...
pos_matrix[pos[0]-1][pos[1]-1]=symbol
...
print('GAME OVER. \n Player with symbol {x} wins!'.format(x=pos_matrix[pos[0]-1][pos[1]-1]))
Why the complex expression to set x
in the format statement? You’ve just assigned symbol
to that pos_matrix entry; just use that!
print('GAME OVER. \n Player with symbol {x} wins!'.format(x=symbol))
Or better, if you are using python 3.6 or later, use an f-string:
print(f'GAME OVER. \n Player with symbol {symbol} wins!')
Your is_win
code is more complex than needed because you are checking if anyone has won, which means a row, column or diagonal has 3 of the same symbol, which is not a space. If you passed the symbol to the function, you can check if that player has won, which means checking for 3 of that symbol only, which is slightly shorter & simpler.
def is_win(symbol):
for i in range(3):
if all( pos_matrix[i][col] == symbol for col in range(3)):
return True
if all( pos_matrix[row][i] == symbol for row in range(3)):
return True
if all( pos_matrix[i][i] == symbol for i in range(3)):
return True
if all( pos_matrix[i][2 - i] == symbol for i in range(3)):
return True
return False
Who's turn is it, anyway?
You call my_tictactoe(...)
and pass in the character to place in the grid. As Hack Saw pointed out, this could be a 'p'
!
Instead, it would be better for the game to keep track of who's turn it is, and automatically use the appropriate symbol.
For example, you could set a global symbol
variable initially to 'x'
, and then have it flip between 'x'
and 'o'
after processing a valid move.
symbol = 'x'
def my_tictactoe(pos):
# ... existing validation, printing, and win detection code ...
# Alternate players: 'x' <--> 'o'
symbol = 'o' if symbol == 'x' else 'x'
As a bonus, after one player wins, the other player will automatically become the first player.
To test the program, with the automatic player tracking, (expanding on Hack Saw's suggestion a bit) the test code could become:
def main():
for pos in ((1,1), (2,2), (3,2), (1,3), (2,1), (3,3), (3,1), (1,2)):
my_tictactoe(pos)
if __name__ == "__main__":
main()