I am a new programmer, learning from youtube and other sites I can get my hands on. The assignment was clear. Create a Tic Tac Toe game using functions as much as possible and where needed.
I am posting this because I am very much interested what this community thinks of this effort and also if there would be any suggestions.
The code is a 2 player game. No AI.
This is what the code does:
- Display welcome and instructions
- Display board
- We start with player X
- While no one has won and it isn't a tie loop
- get player's move
- Update the board with the new move and check legal move, so move that hasn't been taken
- Switch Turns
- Congratulate Winner or say it's a Tie
Actual code:
def show_welcome_message():
"""Display Welcome to Tic Tac Toe """
print(
"""
Hi dear players and welcome to this Tic Tac Toe game_board.
You can win by having three 'X' or three 'O' in a row.
This can be horizontal, vertical or diagonal
"""
)
def show_game_board(game_board):
"""display the board"""
for x in range(3):
print("+---+---+---+" )
for y in range(3):
print("|", game_board[x*3+y], end = " ")
print("|")
print("+---+---+---+" )
def check_winner(game_board):
"""check if there is a win, draw"""
# check rows
for i in range(3):
if game_board[i*3] == game_board[i*3+1] == game_board[i*3+2] and game_board[i*3] in ('X', 'O'):
return game_board[i*3]
# check columns
for i in range(3):
if game_board[i] == game_board[i+3] == game_board[i+6] and game_board[i] in ('X', 'O'):
return game_board[i]
# check diagonals
if game_board[0] == game_board[4] == game_board[8] and game_board[0] in ('X', 'O'):
return game_board[0]
if game_board[2] == game_board[4] == game_board[6] and game_board[2] in ('X', 'O'):
return game_board[2]
# check tie
if all(x in ('X', 'O') for x in game_board):
return 'TIE' #lower case
# no winner or tie
return False
def get_player_move(game_board,user_piece):
""" Get the player's move """
player_move = int(input(f" Please make a move on an unoccupied square player with piece {user_piece} "))
while not is_move_legal(game_board,player_move):
player_move = int(input( " That move is not legal try again "))
return player_move
def is_move_legal(game_board,player_move):
"""Check if the move is legal"""
legal_play = True
if game_board[player_move -1] in ['X','O']:
legal_play = False
return legal_play
def update_game_board(game_board,user_piece,player_move):
""" Modify the board positions into X or O"""
game_board[player_move - 1] = user_piece
def display_game_result(user_piece):
""" display win for X or O or a tie"""
if user_piece == "TIE":
print("This is a TIE players")
else:
print(f"Yes the player with {user_piece} won !!" )
#Addition of stackoverflow
def change_player_turns(user_piece):
""" Change player's piece into X or O"""
return 'O' if user_piece == 'X' else 'X'
def main():
""" game_board starting point """
game_board = [1,2,3,4,5,6,7,8,9]
user_piece = None
show_welcome_message()
show_game_board(game_board)
user_piece = 'X'
print(f"First player starts with {user_piece}")
while not check_winner(game_board):
player_move = get_player_move(game_board,user_piece)
update_game_board(game_board,user_piece,player_move)
show_game_board(game_board)
user_piece = change_player_turns(user_piece)
display_game_result(check_winner(game_board))
if __name__ == "__main__":
main()
-
7\$\begingroup\$ A main guard and no globals! Impressive for a beginner. \$\endgroup\$Reinderien– Reinderien2023年08月24日 13:50:52 +00:00Commented Aug 24, 2023 at 13:50
-
\$\begingroup\$ You do not need the Here is \$\endgroup\$user275173– user2751732023年08月25日 00:04:25 +00:00Commented Aug 25, 2023 at 0:04
-
5\$\begingroup\$ @KimiM We sometimes run into title overlap. There are a lot of Tic Tac Toe games written in python. \$\endgroup\$pacmaninbw– pacmaninbw ♦2023年08月25日 00:33:07 +00:00Commented Aug 25, 2023 at 0:33
3 Answers 3
I would say you did an excellent job overall! The only suggestions I would make are:
The User Is Capable of Entering Unexpected Input
You have:
player_move = int(input( " That move is not legal try again "))
What if the user enters the letter a
? This will prematurely terminate your game with a ValueError
exception.
You should catch this exception. Even if an integer is entered, is it in the range 1..9? If not it could raise an IndexError
when the game board is accessed (e.g. if '10' were entered). The user could also enter '-1', which will not raise an IndexError
, but I don't think you would want to accept this either. So I would explicitly check that the entered value is within the acceptable range.
Avoid Re-calculating the Same Value
For example, you have in function main
:
while not check_winner(game_board):
player_move = get_player_move(game_board,user_piece)
update_game_board(game_board,user_piece,player_move)
show_game_board(game_board)
user_piece = change_player_turns(user_piece)
display_game_result(check_winner(game_board))
After you break out of the while
loop you will make a second call to check_winner
with the same game_board
argument value. You can avoid this redundant and relatively expensive call by using the so-called walrus assignment operator (:=
):
while not (winner := check_winner(game_board)):
player_move = get_player_move(game_board,user_piece)
update_game_board(game_board,user_piece,player_move)
show_game_board(game_board)
user_piece = change_player_turns(user_piece)
display_game_result(winner)
If you do not want to use the walrus assignment operator, then consider:
while True:
winner = check_winner(game_board)
if winner:
break
player_move = get_player_move(game_board,user_piece)
update_game_board(game_board,user_piece,player_move)
show_game_board(game_board)
user_piece = change_player_turns(user_piece)
display_game_result(winner)
Remove Extraneous Spaces From Output
I hesitated to include this section since it is highly subjective and it doesn't go to the heart of what you are trying to accomplish. But for what it's worth:
Your introduction to the players beginning with Hi dear players...
is indented 8 spaces. This is strictly a matter of taste but I don't see that indentation as adding anything. It's not as if you were working with a fixed-width output device and you were attempting to center the text (that would be a different story).
Also, when you solicit a move I see:
+---+---+---+
| 1 | 2 | 3 |
+---+---+---+
| 4 | 5 | 6 |
+---+---+---+
| 7 | 8 | 9 |
+---+---+---+
First player starts with X
Please make a move on an unoccupied square player with piece X
That single-space indentation on the last line should be removed. Perhaps the instruction should read:
Player X, please make a move by specifying the number of an unoccupied square:
Finally:
Bravo! (or should I say: Brava!)
-
1\$\begingroup\$ Thank you so much @Booboo . I will implement these. Haha yes Brava is perfect ! Muchas gracias! \$\endgroup\$LindaJuniordev– LindaJuniordev2023年08月25日 08:48:10 +00:00Commented Aug 25, 2023 at 8:48
Excellent question. I have several suggestions.
- Check indices.
game_board[-1]
is accepted, but you probably don't intend0
to be a move. - To be more robust, use
try/except
to catch errors fromint()
. - Use type hints to help with parameter validation for functions.
- Define functions like
is_move_legal
before you use them for readability. - Add parameter information to your docstrings for readability.
If you wish to extend the project further, here are some ideas:
- What if the number of rows / columns was flexible?
- What if the number of marks-in-a-row to win was flexible?
- What if the number of players was flexible?
- What if the board could have more than 2 dimensions?
- What if the user wanted to play against bot(s)?
I have implemented the suggestions and the first 3 ideas below. Separating features into classes like GameBoard
can make code more modular:
class GameBoard:
def __init__(self, m: int, n: int, win: int):
"""
Creates a board for generalized TicTacToe.
:param m: number of rows
:param n: number of cols
:param win: number of consecutive marks to win
"""
if m < 1:
raise ValueError("Number of rows must be positive!")
if n < 1:
raise ValueError("Number of cols must be positive!")
if win < 1:
raise ValueError("Marks per win must be positive!")
self.m = m
self.n = n
self.win = win
# board[i][j] is None if empty or a player's mark
self.board = [[None] * n for _ in range(m)]
def __str__(self):
"""
Returns this object in string form.
:return: a regular grid of entries
"""
m = self.m
n = self.n
# max number of characters per entry
max_width = len(str(m * n))
row_div = "+" # divider per row
for j in range(n):
row_div += ("-" * max_width + "--+")
# build the board as a string
result = row_div
for i in range(m):
result += "\n"
for j in range(n):
value = self.board[i][j]
if value is None:
value = i * n + j + 1
display = str(value)
blanks = " " * (max_width - len(display))
result += f"| {display}{blanks} "
result += "|\n" + row_div
return result
def move_worked(self, move_input: str, player: str):
"""
Attempt a move and return whether it worked.
:param move_input: the player's input
:param player: the player's mark (e.g. X, O)
:return: whether the move was valid and played
"""
try:
move = int(move_input) - 1
except ValueError:
return False
m = self.m
n = self.n
i = move // n
j = move % n
if 0 <= i < m and 0 <= j < n and self.board[i][j] is None:
self.board[i][j] = player
return True
return False
def get_winner(self):
"""
Returns the first winner found (or None if no winners).
:return: string or None
"""
m = self.m
n = self.n
win = self.win
board = self.board
for i in range(m):
for j in range(n):
# leftmost top mark of the pattern
mark = board[i][j]
if mark is None:
continue
if j + win <= n:
# row
if all(board[i][j + k] == mark for k in range(win)):
return mark
if i + win <= m:
# column
if all(board[i + k][j] == mark for k in range(win)):
return mark
# top-left to bottom-right diagonal
if j + win <= n:
if all(board[i + k][j + k] == mark for k in range(win)):
return mark
# top-right to bottom-left diagonal
if j + 1 >= win:
if all(board[i + k][j - k] == mark for k in range(win)):
return mark
# no winner yet
return None
def display_game_result(winner):
"""
Displays the outcome of a game
:param winner: string representing player's mark
"""
if winner is None:
print("It's a tie!")
else:
print(f"Player {winner} won!!!")
def main():
m = 7
n = 5
win = 5
user_pieces = ['X', 'O', 'Y']
print(f"""
Welcome to Tic Tac Toe! Our board is {m} x {n}.
You can win by having {win} marks in a line.
Lines can be horizontal, vertical or diagonal.
""")
game_board = GameBoard(m, n, win)
turn_count = 0
# after m * n moves without a winner, it's a tie
while game_board.get_winner() is None and turn_count < m * n:
print(game_board)
user_piece = user_pieces[turn_count % len(user_pieces)]
turn_count += 1
print(f"Turn {turn_count}: {user_piece} to play ...")
player_move = input("Please enter the number of the square to mark: ")
while not game_board.move_worked(player_move, user_piece):
player_move = input("That move is not legal, try again. ")
print(game_board)
display_game_result(game_board.get_winner())
if __name__ == "__main__":
main()
Keep up the great work!
Missing input validation
The user can input any text, only some of them will be successfully converted to int
, and even fewer is valid (x in range(1, 10)
). What if the user inputs cat
, what happens then? A ValueError
exception is thrown, because 'cat'
cannot be converted to an integer (unless you are converting a numeral from a base no less than 30, cat30 = 1112910, in this case you need to specify base
parameter in int
call: int('cat', 30) == 11129
):
In [76]: int('cat')
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
Cell In[76], line 1
----> 1 int('cat')
ValueError: invalid literal for int() with base 10: 'cat'
And for numbers not in range(1, 10)
, your code will cause IndexError
:
First player starts with X
Please make a move on an unoccupied square player with piece X 11
IndexError: list index out of range
I have refactored your function to get user input and added input validation:
import contextlib
def get_player_move(game_board, user_piece):
""" Get the player's move """
while True:
choice = -1
while not 0 < choice < 10:
while True:
with contextlib.suppress(ValueError):
choice = int(input(f" Please make a move on an unoccupied square player with piece {user_piece} "))
break
if game_board[choice -1] not in {'X','O'}:
break
return choice
Complicated code to check winner
Your code to check winner is very complicated, and much more complicated than it should be.
There is a lot of repetition in the code, the conditions to check for columns, rows and diagonals are almost identical, except for the indices.
(0, 1, 2), (3, 4, 5), (6, 7, 8)
indices are on the same row, (0, 3, 6), (1, 4, 7), (2, 5, 8)
indices are on the same column. (0, 4, 8), (2, 4, 6)
are the diagonals.
How to get values at those indices? Notice in each group, the difference between two consecutive indices are the same? For indices in the same row, the step is 1, in the same column the step is 3, then for one diagonal the step is 4 for the other diagonal the step is 2.
In Python we use range
to generate numbers, range(start, stop, step)
will generate the following numbers: start, start + step, start + 2 * step ... (stop)
, the stop
value is not included. For example, list(range(0, 3, 1)) == [0, 1, 2]
.
We can use slicing to get values located at those indices, for example, list(range(1, 10)[0:9:4]) == [1, 5, 9]
.
We can store the start and stop indices and the steps in a nested tuple
and just then iterate through it to check all conditions.
LINES = (
(0, 3, 1),
(3, 6, 1),
(6, 9, 1),
(0, 7, 3),
(1, 8, 3),
(2, 9, 3),
(0, 9, 4),
(2, 7, 2)
)
Now how do we check if all elements are equal? In Python, elements of a set
are unique, so if len(set(collection)) == 1
, then all elements in collection
are the same.
def check_winner(game_board):
"""check if there is a win, draw"""
for start, stop, step in LINES:
if len(set(game_board[start:stop:step])) == 1 and (winner := game_board[start]) in {'X', 'O'}:
return winner
return 'TIE' if all(x in ('X', 'O') for x in game_board) else False
-
\$\begingroup\$ Thank you so much! It gives me a new insight in how to achieve this \$\endgroup\$LindaJuniordev– LindaJuniordev2023年08月25日 13:05:03 +00:00Commented Aug 25, 2023 at 13:05
-
\$\begingroup\$ I wouldn't say it's good advice for a beginner to merge small functions into bigger ones. A codebase with (sensible) small functions is often easier to read and definitely easier to unit-test. OP struck a decent balance here. I like your rewrite of the win condition though. \$\endgroup\$Seb– Seb2023年08月25日 15:41:54 +00:00Commented Aug 25, 2023 at 15:41