3
\$\begingroup\$

I have written a simple Tic Tac Toe game using some ASCII art. Is there anything I can do to improve upon the logic and structure of the program?

"""A basic command line tic tac toe game"""
import os
import sys
gameBoard = [['' for j in range(3)] for i in range(3)]
displayBoard = [[' ' for j in range(46)] for i in range(25)]
def main():
 """Main method to control game"""
 player = 'X'#Player x to go first
 moveCounter = 1 #Keeps track of how many turns have been taken
 #Setup game
 initGame()
 printInitScreen()
 while True:
 #Get player input
 square = input(f"Player {player}, choose your square ->")
 if validateInput(square):
 updateGameBoard(square, player)
 updateDisplayBoard(square, player)
 printDisplayBoard()
 if moveCounter >= 4:
 checkIfWon(player)
 #Switch player
 if player == 'X':
 player = 'O'
 else:
 player = 'X'
 moveCounter += 1
 else:
 print("Please try again")
def initGame():
 """Create and set up game components"""
 #Fill board
 for i in range(25):
 #If on a row boarder set to _
 if i == 8 or i == 17:
 for j in range(46):
 displayBoard[i][j] = '_'
 else:
 for j in range(46):
 #If on column boarder set |
 if j == 15 or j == 31:
 displayBoard[i][j] = '|'
 #Put numbers in corner of square
 displayBoard[0][0] = '1'
 displayBoard[0][16] = '2'
 displayBoard[0][32] = '3'
 displayBoard[9][0] = '4'
 displayBoard[9][16] = '5'
 displayBoard[9][32] = '6'
 displayBoard[18][0] = '7'
 displayBoard[18][16] = '8'
 displayBoard[18][32] = '9'
def validateInput(input):
 """Validates user input"""
 #Check given char is allowed
 try:
 square = int(input[0]) #Check first char of input is number
 except:
 return False
 #Check nothing already in that square
 #Get the gameBoard index of users chosen square
 index = numToIndex(input)
 if gameBoard[index[0]][index[1]] != '':
 return False
 #If all ok
 return True
def updateGameBoard(input, player):
 """Keeps track of users moves"""
 #Update the array with new move
 index = numToIndex(input[0])
 gameBoard[index[0]][index[1]] = player
def printDisplayBoard():
 """Prints a string representation of board"""
 os.system('cls' if os.name == 'nt' else 'clear') # Clear screen
 for row in displayBoard:
 print(''.join(row))
 print("")
def checkIfWon(player):
 """Checks to see if the last move won the game"""
 gameWon = False
 #Check Horiz
 for row in gameBoard:
 if row[0] == row[1] == row[2] == player:
 gameWon = True
 #Check Vert
 for i in range(3):
 if gameBoard[0][i] == gameBoard[1][i] == gameBoard[2][i] == player:
 gameWon = True
 #Check Diag
 if gameBoard[0][0] == gameBoard[1][1] == gameBoard[2][2] == player:
 gameWon = True
 if gameBoard[0][2] == gameBoard[1][1] == gameBoard[2][0] == player:
 gameWon = True
 if gameWon:
 print(f"Congratualtions player {player}, you won!")
 sys.exit()
def printGameBoard():
 """For debugging, prints gameboard"""
 for row in gameBoard:
 print(row)
def printInitScreen():
 """Prints the initial welcome screen"""
 header = """
 888 d8b 888 888
 888 Y8P 888 888
 888 888 888
 888888888 .d8888b888888 8888b. .d8888b888888 .d88b. .d88b.
 888 888d88P" 888 "88bd88P" 888 d88""88bd8P Y8b
 888 888888 888 .d888888888 888 888 88888888888
 Y88b. 888Y88b. Y88b. 888 888Y88b. Y88b. Y88..88PY8b.
 "Y888888 "Y8888P "Y888"Y888888 "Y8888P "Y888 "Y88P" "Y8888
 """
 os.system('cls' if os.name == 'nt' else 'clear') # Clear screen
 print(header)
 input("Press enter to start!")
 printDisplayBoard()
def updateDisplayBoard(num, player):
 """Add the players shape to the chosen position on display board"""
 shapes = {"X":
 [[' ',' ',' ',' ','?','8','8','8','8','P',' ',' ',' ',' '],
 [' ',' ',' ',' ',' ','`','8','8','`',' ',' ',' ',' ',' '],
 ['8','b',',','_',' ',' ','8','8',' ',' ','_',',','d','8'],
 ['8','8','8','8','8','S','I','C','K','8','8','8','8','8'],
 ['8','P','~',' ',' ',' ','8','8',' ',' ',' ','~','?','8'],
 [' ',' ',' ',' ',' ',',','8','8','.',' ',' ',' ',' ',' '],
 [' ',' ',' ',' ','d','8','8','8','8','b',' ',' ',' ',' ']],
 "O":
 [[' ',' ',' ',' ',' ',' ','%','%',' ',' ',' ',' ',' ',' '],
 [' ',' ',' ',' ','%','%',' ',' ','%','%',' ',' ',' ',' '],
 [' ',' ','%','%',' ',' ',' ',' ',' ',' ','%','%',' ',' '],
 ['%','%',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ','%','%'],
 [' ',' ','%','%',' ',' ',' ',' ',' ',' ','%','%',' ',' '],
 [' ',' ',' ',' ','%','%',' ',' ','%','%',' ',' ',' ',' '],
 [' ',' ',' ',' ',' ',' ','%','%',' ',' ',' ',' ',' ',' ']]}
 shape = shapes[player]
 num = int(num[0])
 offsets = [[0 ,0],[0 ,16],[0 ,32],
 [9 ,0],[9 ,16],[9 ,32],
 [17,0],[17,16],[17,32]]
 iOffset = offsets[num-1][0]
 jOffset = offsets[num-1][1]
 for i in range(iOffset, iOffset + 7):
 for j in range(jOffset, jOffset + 14):
 displayBoard[i+1][j] = shape[i - iOffset][j - jOffset]
def numToIndex(num):
 """Returns index [i,j] for given 'square' on board"""
 num = int(num[0])
 indexList = []
 for i in range (3):
 for j in range(3):
 indexList.append([i,j])
 return indexList[int(num)-1]
if __name__ == '__main__':
 main()
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Apr 28, 2020 at 6:57
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$
gameBoard = [['' for j in range(3)] for i in range(3)]
displayBoard = [[' ' for j in range(46)] for i in range(25)]

For me, time spend aligning source code is time wasted. It takes too much time initially. Moreover, if any variable is renamed throughout the code (or cleaned up etc. etc.), the aligning will get messed up. So it actively interferes with refactoring. This is highly subjective though, and I like to look at aligned code.


gameBoard = [['' for j in range(3)] for i in range(3)]

A game board (or game board state, more precisely) consists of squares, filled with X'es or O's. What's missing is an object to represent those. For me, this is a bit too "stringly typed".

displayBoard = [[' ' for j in range(46)] for i in range(25)]

This is the start of a problem in the code: unexplained numeric literals. Why 46 or 25? Just perform the calculations and let the computer compute. Make named constants out of the calculations and then use those.

player = 'X'#Player x to go first

If you name the identifier currentPlayer then the comment would not be necessary. Often choosing a good name will make comments unnecessary.

I'd try and avoid trailing comments as they tend to disappear on long lines. Again, this is somewhat subjective.

Use spaces before and after # please.

updateGameBoard(square, player)
updateDisplayBoard(square, player)

I spot a design issue here. The UI should display the state of the game, captured in gameBoard. So I would expect a printDisplayBoard(gameBoard) instead. Currently the game logic and UI is mixed and duplicated.

if moveCounter >= 4:
 checkIfWon(player)

This is not so intuitive to me. The game ends when a row, column or diagonal of three elements is formed. Currently you may have two winning players! If the board is filled without this happening then the game is a draw.

Deviating from this game logic is not a good idea as it is easy to make mistakes. Moreover, and more importantly, it fails if the application is ever extended to, for instance, a 4x4 board. Now I don't expect "tic tac toe too" to happen soon, but for actual application code, this is rather important.

My SudokuSolver was programmed as generically as possible. When I was done it literally took a minute to support differently sized and even special Sudoku's with additional diagonals and such. This shows the importance of programming as generically as possible.

def initGame():

I'd create drawHorizontalLine and drawVertialLine functions or similar.

displayBoard[0][0] = '1'
displayBoard[0][16] = '2'
displayBoard[0][32] = '3'

I like how you clearly mark the squares of the board. However, this also is one of the clearest examples of you doing the computing instead of the computer. It should be relatively easy to create a single for loop and compute the x and y of the positions.

Basically you're buffering the image before displaying it, rather than making a single print with spaghetti code. That's very good.

os.system('cls' if os.name == 'nt' else 'clear') # Clear screen
.
.
.
os.system('cls' if os.name == 'nt' else 'clear') # Clear screen

Repeat 10 times: "I will keep to DRY principles".

If you repeat such a line, then put the line in a function and call that. If you add another method of clearing the screen then likely one of the locations with the same line of code will be forgotten. Looking at the comment, you've already thought of a name and started typing.

Of course, there is a lot of DRY (don't repeat yourself) failures where the same code is used but with different integer values, but this one stood out.

shapes = [...]

Do you really want that variable to be assigned the entire shape all the time? That needs to be a constant or - in real code - a resource that is being read once.

num = int(num[0])

Wait, what? Why? If the reason for code is not immediately clear, then I expect a comment!

offsets = [[0 ,0],[0 ,16],[0 ,32],
 [9 ,0],[9 ,16],[9 ,32],
 [17,0],[17,16],[17,32]]

Which offsets would that be? End of code, must be Friday :)

def numToIndex(num):

Finally, a function that calculates things! I knew you could do it.

answered Apr 28, 2020 at 9:06
\$\endgroup\$
2
  • \$\begingroup\$ Hi, thank you for taking the time to read through and help me with my code. I have learnt a lot from your answer. I'm sometimes cautious of using functions where maybe I don't need to, but you've cleared some things up for me. printDisplayBoard(gameBoard) This especially makes perfect sense now I have seen it. \$\endgroup\$ Commented Apr 28, 2020 at 9:44
  • \$\begingroup\$ Some people try and create methods with just a few lines, say 5-7 (for a higher level language). That's a bit too extreme for me, but in general large functions / methods are more troublesome than shorter ones, especially if they use a lot of branching (loops, if statements and switches). This is referred to as the cyclomatic complexity of a function. Programming is basically fighting complexity. \$\endgroup\$ Commented Apr 28, 2020 at 10:26

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.