Simple Python 3 TicTacToe Game

Question

Relatively new here so please let me know if I'm doing something wrong.

I'm mostly wondering if there's any additional way I can make this code more efficient. Specifically the moveset and win condition part of the code feels unnecessarily long or overcomplicated. Still a beginner in python, so any feedback regarding syntax is also appreciated! Thank you!

#Contains all basic variables for ease of coding
class Player():
 symbols = ['O','X']
 def __init__(self, number):
 self.wins = 0
 self.number = number
 self.symbol = self.symbols[number]
#2D list to represent the 3x3 board 
def createBoard():
 a = ['', '', '']
 b = ['', '', '']
 c = ['', '', '']
 return [a, b, c]
board = createBoard()
#Makes the board look nicer for user; numbers as placeholder for empty spaces
def printBoard():
 i = 0
 for row in board:
 print("---------------")
 for place in row:
 i += 1
 if place == '':
 print("|",i,"|", end='')
 else:
 print("|",place,"|", end='')
 print("")
 print("---------------")
#Receives numbers to return x and y position of the 2d list
def getMove():
 #Any other more efficient way for this?
 moveset = {
 '1': (0,0), 
 '2': (1,0),
 '3': (2,0),
 '4': (0,1),
 '5': (1,1),
 '6': (2,1),
 '7': (0,2),
 '8': (1,2),
 '9': (2,2)
 }
 print("Please enter the number of the spot to place your move",end='')
 xpos, ypos = moveset[input("> ")]
 return xpos, ypos
#changes the board with received x and y
def placeMove(xpos, ypos, player):
 global board
 board[ypos][xpos] = player.symbol
#Checkes if the spot has been filled 
def validateMove(xpos, ypos):
 if board[ypos][xpos] != '':
 print("Cannot place there!")
 return False
 return True
#Checks win conditions; any more efficient way for this as well?
def checkWin():
 for row in board:
 if row[0]==row[1]==row[2]:
 return row[0]
 for i in range(0,3):
 if board[0][i]==board[1][i]==board[2][i]:
 return board[0][i]
 if board[0][0]==board[1][1]==board[2][2]:
 return board[1][1]
 elif board[0][2]==board[1][1]==board[2][0]:
 return board[1][1]
 for row in board:
 for spot in row:
 if spot == '':
 return False
 return 2
def switchPlayer(currentPlayer):
 if currentPlayer.number == 0:
 return 1
 else:
 return 0
#game loop
def game():
 global board
 playerList = [Player(0), Player(1)]
 game = True
 currentPlayer = playerList[0] 
 printBoard()
 while game: 
 print("Current Player:",currentPlayer.number+1,)
 xpos, ypos = getMove()
 #repeats until valid input
 while not validateMove(xpos, ypos):
 xpos, ypos = getMove()
 placeMove(xpos, ypos, currentPlayer)
 printBoard()
 if checkWin() == currentPlayer.symbol:
 print("Player",currentPlayer.number+1, "wins!")
 game = False
 elif checkWin() == 2:
 print("The game is a draw!")
 game = False
 else:
 currentPlayer = playerList[switchPlayer(currentPlayer)]
#Should I just make the game() the main() function?
def main():
 game()
if __name__ == "__main__":
 main()

• glenn jackman: Not an answer, but you might want to read rosettacode.org/wiki/Tic-tac-toe#Python to see how others did it.

Answer 1

In addition to Reinderien's comments:

---

checkWin() first checks board[0][0]==board[0][1]==board[0][2] and returns the "winning" symbol (a string), if the match was found.

This means that, with the following board:

 | | 
---+---+---
 X | X | 
---+---+---
 O | O | O 

checkWin() returns '', and checkWin() == currentPlayer.symbol is false, so "O" doesn't win. And "X" can fill their row on their move, and will be declared the winner in two more moves!

The only player who can win, is the one that just made a move, so pass that player's symbol to the checkWin() call, and explicitly check for that player winning:

def checkWin(symbol):
 for row in board:
 if row[0] == row[1] == row[2] == symbol:
 return symbol

---

checkWin() returns 3 different types of values!

• str if a winner is detected
• bool if there are still moves which can be made
• int if the game is tied.

This is just plain wrong. Pick one return type. For instance, you could use int, and return 0 for game not over, 1 for a win by the current player, and -1 for a draw game.
Or you could use str, and return "X" or "O" for a win by that player, "Cat's game" for a draw, and "" for the game not over. Then, created named constants for the values, and use those names in your code, instead of the actual values. Eg)

GAME_HAS_WINNER = 1
GAME_IS_A_DRAW = -1
GAME_IS_NOT_OVER = 0
# ...
def game():
 # ...
 while True:
 # ...
 state = checkWin(currentPlayer.symbol)
 if state == GAME_HAS_WINNER:
 # ... declare winner
 break
 elif state == GAME_IS_A_DRAW:
 # ... declare a draw
 break
 else:
 # ...

Better would be to create an Enum for the return code, if you've learnt that.

---

You can simplify your createBoard() & printBoard() functions, and at the same time fix the bug in checkWin() by initializing the board with the number characters, instead of empty strings.

def createBoard():
 return [['1', '2', '3'], ['4', '5', '6'], ['7', '8', '9']]
def printBoard():
 for row in board:
 print("---------------")
 for place in row:
 print("|", place, "|", end='')
 print("")
 print("---------------")

You'll need a different way to check for a draw game. The simplest would be to remove that test from checkWin() and change your while loop into a for loop that runs for at most 9 turns. If you finish the loop without break-ing out of the loop, the else: clause gets executed.

def game():
 # ...
 for _ in range(9):
 # ...
 if checkWin(currentPlayer.symbol) == GAME_HAS_WINNER:
 # ... announce winner
 break
 # ...
 else:
 # ... announce draw game

---

You can use list comprehension to simplify checkWin(). For instance, checking only for a win by a given symbol, and returning True for a win, and False otherwise:

def checkWin(symbol):
 if any( all(board[i][j] == symbol for i in range(3)) for j in range(3)):
 return True
 if any( all(board[i][j] == symbol for j in range(3)) for i in range(3)):
 return True
 if all(board[i][i] == symbol for i in range(3)):
 return True
 return all(board[i][2-i] == symbol for i in range(3)):

Answer 2

symbols = ['O','X']

This will not change, so it doesn't need to be mutable. It's better represented as a string:

symbols = 'OX'

This:

def createBoard():
 a = ['', '', '']
 b = ['', '', '']
 c = ['', '', '']
 return [a, b, c]

can simply be

def create_board():
 return [['']*3 for _ in range(3)]

Note that the convention for Python is snake_case, not camelCase.

getMove can be made more efficient. You do not need a dictionary.

i = int(input('Please enter the number of the '
 'spot to place your move > ')) - 1
return i%3, i//3

You're using board as a global, which is a code smell. Most of these methods, along with the board member, should be put into a class.

This:

if currentPlayer.number == 0:
 return 1
else:
 return 0

can be:

return 1 - current_player.number

Your boolean game variable is unnecessary. Rather than setting it to false, simply break out of the loop.

At the top of the file, there should be a shebang:

#!/usr/bin/env python3