patternpythonMinor
Basic Python Tic-Tac-Toe Game
Viewed 0 times
toetictacgamepythonbasic
Problem
I have just decided to get into programming as a hobby (and possibly a job in the distant future). After learning the basics of Python, I made Tic-Tac-Toe to apply what I've learned. I would appreciate any and all feedback on my code and the way I set up the logic of the game. I would like to avoid picking up bad coding habits.
```
from random import randint
from time import sleep
from copy import deepcopy
#Setup game
def main():
#Set default board values
global board
board = [[0, " 0 "], [1, " 1 "], [2," 2 "], [3," 3 "], [4," 4 "], [5," 5 "], [6," 6 "], [7," 7 "], [8," 8 "]]
#Sets default counter value:
global counter
counter = 0
#Calls function to draw board
drawBoard()
#Draw game board
def drawBoard():
#Draws game board with proper variables
print(board[0][1] + "|"+ board[1][1] +"|" + board[2][1] + "\n---+---+--- \n" + board[3][1] +"|" + board[4][1] + "|"
+ board[5][1] + "\n---+---+--- \n" + board[6][1] + "|" + board[7][1] + "|" + board[8][1] + "\n")
#After board has been updated, initiates next turn
nextTurn(counter)
#Execute the next turn
def nextTurn(count):
#If count is divisible by two, let the player make a move and increase turn counter
if count % 2 == 0:
global counter
counter += 1
playerMove()
#If count is not divisible by two, let the cmputer make a move and increase turn counter
else:
counter += 1
computerMove()
#Code for the player's move
def playerMove():
#Get player input and try to convert it to an integer. If it cannot be, prompt user to enter again
try:
boardSpot = int(input("It's your turn! Please select an empty space on the board by typing a number 1-9\n"))
except ValueError:
playerMove()
#If the space is empty, proceed
if board[boardSpot][1] != " X " and board[boardSpot][1] != " O " :
#Set board spot to X and redraw the board
board[boardSpot][1] = " X "
if check
```
from random import randint
from time import sleep
from copy import deepcopy
#Setup game
def main():
#Set default board values
global board
board = [[0, " 0 "], [1, " 1 "], [2," 2 "], [3," 3 "], [4," 4 "], [5," 5 "], [6," 6 "], [7," 7 "], [8," 8 "]]
#Sets default counter value:
global counter
counter = 0
#Calls function to draw board
drawBoard()
#Draw game board
def drawBoard():
#Draws game board with proper variables
print(board[0][1] + "|"+ board[1][1] +"|" + board[2][1] + "\n---+---+--- \n" + board[3][1] +"|" + board[4][1] + "|"
+ board[5][1] + "\n---+---+--- \n" + board[6][1] + "|" + board[7][1] + "|" + board[8][1] + "\n")
#After board has been updated, initiates next turn
nextTurn(counter)
#Execute the next turn
def nextTurn(count):
#If count is divisible by two, let the player make a move and increase turn counter
if count % 2 == 0:
global counter
counter += 1
playerMove()
#If count is not divisible by two, let the cmputer make a move and increase turn counter
else:
counter += 1
computerMove()
#Code for the player's move
def playerMove():
#Get player input and try to convert it to an integer. If it cannot be, prompt user to enter again
try:
boardSpot = int(input("It's your turn! Please select an empty space on the board by typing a number 1-9\n"))
except ValueError:
playerMove()
#If the space is empty, proceed
if board[boardSpot][1] != " X " and board[boardSpot][1] != " O " :
#Set board spot to X and redraw the board
board[boardSpot][1] = " X "
if check
Solution
First:
You've put your logic into a function rather than have it at the module level. This is a good thing (tm)!
Some style things:
If you're beginning, PEP 8, the official style guide, is a must read. A few things:
Maybe an 80 column max is a bit conservative with todays screens, but you get up to column 120 with code and 122 with comments. Anything further than column 100 is probably pushing standard width, especially with Python.
This is a big one for readability. You have no blank lines in your source code, so everything is just sort of all squished together.
Docstrings are described in PEP 257. These can and should replace your comments before the methods, as support is build into the language for them.
Python recommends using
I would print the board after the final move has been made.
And one VERY opinionated suggestion: Your input is a 3x3 grid of numbers. Use the 3x3 grid of numbers that exist on most full-size keyboards: the numpad:
Now on to looking at the code itself:
Since you're just beginning, I'll let global slide. For the most part you want to avoid using global data as it can easily lead to problems, but for
Why are you using a two dimensional list here? You never access the first element of each sublist after assigning it. And since you're putting the numbers in ascending order anyway, you can use a List Comprehension to accomplish the declaration simply:
If you use the numpad-ordering, using a list comprehension becomes harder and not worth the effort.
Now, since we've changed
Even with removing the second layer of list, that's still a doozy to read. Using String Formatting, I think we can clean this up a bit. Along with multi-line strings, we get
One thing remains to be explained here: I'm using the
This breaks the idea of one method should do one thing. Effectively, your draw call also starts the next turn. And since the
I don't think you need it. Rather than recursively call
I would have
One function should have one job. That's the biggest issue here is that (nearly) every function here does one thing, then another thing.
One final thought:
That is a mess. First of all, I don't even see why you pass in
if __name__ == "__main__":
main()
You've put your logic into a function rather than have it at the module level. This is a good thing (tm)!
Some style things:
If you're beginning, PEP 8, the official style guide, is a must read. A few things:
- Limit all lines to a maximum of 79 characters.
Maybe an 80 column max is a bit conservative with todays screens, but you get up to column 120 with code and 122 with comments. Anything further than column 100 is probably pushing standard width, especially with Python.
- Method definitions inside a class are surrounded by a single blank line.
This is a big one for readability. You have no blank lines in your source code, so everything is just sort of all squished together.
Docstrings are described in PEP 257. These can and should replace your comments before the methods, as support is build into the language for them.
Python recommends using
lower_case_with_underscores to name your functions.I would print the board after the final move has been made.
And one VERY opinionated suggestion: Your input is a 3x3 grid of numbers. Use the 3x3 grid of numbers that exist on most full-size keyboards: the numpad:
789 / 456 / 123Now on to looking at the code itself:
global
Since you're just beginning, I'll let global slide. For the most part you want to avoid using global data as it can easily lead to problems, but for
board here it's the easy way to do it. I'd get rid of counter, but I'll get to that in a few paragraphs.board = [[0, " 0 "], [1, " 1 "], [2," 2 "], [3," 3 "], [4," 4 "], [5," 5 "], [6," 6 "], [7," 7 "], [8," 8 "]]
Why are you using a two dimensional list here? You never access the first element of each sublist after assigning it. And since you're putting the numbers in ascending order anyway, you can use a List Comprehension to accomplish the declaration simply:
board = [' ' + str(i) + ' ' for i in range(9)]
If you use the numpad-ordering, using a list comprehension becomes harder and not worth the effort.
Now, since we've changed
board, we need to fix drawBoard():print(board[0] + "|"+ board[1] +"|" + board[2] + "\n---+---+--- \n" + board[3] +"|" + board[4] + "|"
+ board[5] + "\n---+---+--- \n" + board[6] + "|" + board[7] + "|" + board[8] + "\n")
Even with removing the second layer of list, that's still a doozy to read. Using String Formatting, I think we can clean this up a bit. Along with multi-line strings, we get
"""\
{0}|{1}|{2}
---+---+---
{3}|{4}|{5}
---+---+---
{6}|{7}|{8}""".format(*board)
One thing remains to be explained here: I'm using the
*board to unpack board into the arguments. Basically, it's the same as board[0],board[1],board[2],etc.def drawBoard():
[...]
nextTurn(counter)
This breaks the idea of one method should do one thing. Effectively, your draw call also starts the next turn. And since the
nextTurn call leads to drawing the board again, you've got a recursive loop here that doesn't need to happen, and doesn't make sense, if drawBoard() does what you claim it to do: draw the game board. To address this loop, I move on to the next point:global counter
I don't think you need it. Rather than recursively call
nextTurn with an increasing turn counter, define a game_loop() function. Call it from main() after you set up, and loop back and forth between player and AI turns. It would look something like this:def game_loop():
while True:
playerMove()
if game_has_ended(): break
computerMove()
if game_has_ended(): break
endOfGame()
I would have
game_has_ended() be a simple check to perform in the loop here, then endOfGame() would be refactored to include the printing of congratulations to the winning team.One function should have one job. That's the biggest issue here is that (nearly) every function here does one thing, then another thing.
One final thought:
return ((brd[6][1] == lttr and brd[7][1] == lttr and brd[8][1] == lttr) or # across bottom
(brd[3][1] == lttr and brd[4][1] == lttr and brd[5][1] == lttr) or # across middle
(brd[0][1] == lttr and brd[1][1] == lttr and brd[2][1] == lttr) or # across top
(brd[6][1] == lttr and brd[3][1] == lttr and brd[0][1] == lttr) or # down left side
(brd[7][1] == lttr and brd[4][1] == lttr and brd[1][1] == lttr) or # down middle
(brd[8][1] == lttr and brd[5][1] == lttr and brd[2][1] == lttr) or # down right side
(brd[6][1] == lttr and brd[4][1] == lttr and brd[2][1] == lttr) or # diagonal
(brd[8][1] == lttr and brd[4][1] == lttr and brd[0][1] == lttr)) # diagonal
That is a mess. First of all, I don't even see why you pass in
brd as an alias for board if board is global; you might as well just use it. Secondly, that's a lot of hard coded cases. Unfortunately, I don't see an easy way to clean it up, but using the commutative property of equality we can clean it up a little, at the very least. Here's a modified version:Context
StackExchange Code Review Q#123763, answer score: 2
Revisions (0)
No revisions yet.