12
\$\begingroup\$

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()
Sᴀᴍ Onᴇᴌᴀ
29.5k16 gold badges45 silver badges201 bronze badges
asked Aug 24, 2023 at 12:46
\$\endgroup\$
3
  • 7
    \$\begingroup\$ A main guard and no globals! Impressive for a beginner. \$\endgroup\$ Commented Aug 24, 2023 at 13:50
  • \$\begingroup\$ You do not need the Here is \$\endgroup\$ Commented 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\$ Commented Aug 25, 2023 at 0:33

3 Answers 3

8
\$\begingroup\$

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!)

answered Aug 24, 2023 at 22:06
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you so much @Booboo . I will implement these. Haha yes Brava is perfect ! Muchas gracias! \$\endgroup\$ Commented Aug 25, 2023 at 8:48
5
\$\begingroup\$

Excellent question. I have several suggestions.

  • Check indices. game_board[-1] is accepted, but you probably don't intend 0 to be a move.
  • To be more robust, use try/except to catch errors from int().
  • 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!

answered Aug 25, 2023 at 1:41
\$\endgroup\$
0
2
\$\begingroup\$

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
answered Aug 25, 2023 at 8:07
\$\endgroup\$
2
  • \$\begingroup\$ Thank you so much! It gives me a new insight in how to achieve this \$\endgroup\$ Commented 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\$ Commented Aug 25, 2023 at 15:41

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.