2
\$\begingroup\$

Hey guys this is my second attempt at a Tic Tac Toe game using the advice given in my first post: My first Python project: Tic Tac Toe

Again, just looking for advice on things I can improve. There is no classes because I have not really learned a great deal about OOP yet. I wanted to make sure I could use functions proficiently before I continued. I'm sure there is a way to loop the if statements in check_win(), but I have not figured it out yet.

def num_to_coord(num):
# 0-index num
    num -= 1
    coord = []
    while True:
        curr_coord = num % 3
        coord.append(curr_coord)
        if len(coord) >= 2:
            break
        num -= curr_coord
        num //= 3
    return coord


def change_symbol(the_symbol):
    if the_symbol == 'x':
        the_symbol = 'o'
        return the_symbol
    if the_symbol == 'o':
        the_symbol = 'x'
        return the_symbol


def get_position():
    position = int(input("Enter a position: "))
    return position


def check_position(the_position, the_taken_positions):
    if the_position in the_taken_positions:
        return True
    if the_position not in the_taken_positions:
        return False


def draw_position(location, the_symbol, the_game_board):
    coord = num_to_coord(location)
    the_game_board[coord[0]][coord[1]] = the_symbol
    for y in range(3):
        print(add_sep('|', [the_game_board[x][y] for x in range(3)]))


def add_sep(sep, lst):
    """Adding | to board"""
    output = sep
    for i in lst:
        output += i + sep
    return output


def check_win(the_game_board):
    if the_game_board[0][0] == 'x' and the_game_board[1][0] == 'x' and the_game_board[2][0] == 'x' or \
        (the_game_board[0][0] == 'o' and the_game_board[1][0] == 'o' and the_game_board[2][0] == 'o'):
        return True
    if the_game_board[0][1] == 'x' and the_game_board[1][1] == 'x' and the_game_board[2][1] == 'x' or \
        (the_game_board[0][1] == 'o' and the_game_board[1][1] == 'o' and the_game_board[2][1] == 'o'):
        return True
    if the_game_board[0][2] == 'x' and the_game_board[1][2] == 'x' and the_game_board[2][2] == 'x' or \
        (the_game_board[0][2] == 'o' and the_game_board[1][2] == 'o' and the_game_board[2][2] == 'o'):
        return True
    if the_game_board[0][0] == 'x' and the_game_board[0][1] == 'x' and the_game_board[0][2] == 'x' or \
        (the_game_board[0][0] == 'o' and the_game_board[0][1] == 'o' and the_game_board[0][2] == 'o'):
        return True
    if the_game_board[1][0] == 'x' and the_game_board[1][1] == 'x' and the_game_board[1][2] == 'x' or \
        (the_game_board[1][0] == 'o' and the_game_board[1][1] == 'o' and the_game_board[1][2] == 'o'):
        return True
    if the_game_board[2][0] == 'x' and the_game_board[2][1] == 'x' and the_game_board[2][2] == 'x' or \
        (the_game_board[2][0] == 'o' and the_game_board[2][1] == 'o' and the_game_board[2][2] == 'o'):
        return True
    if the_game_board[0][0] == 'x' and the_game_board[1][1] == 'x' and the_game_board[2][2] == 'x' or \
        (the_game_board[0][0] == 'o' and the_game_board[1][1] == 'o' and the_game_board[2][2] == 'o'):
        return True
    if the_game_board[0][2] == 'x' and the_game_board[1][1] == 'x' and the_game_board[2][0] == 'x' or \
        (the_game_board[0][2] == 'o' and the_game_board[1][1] == 'o' and the_game_board[2][0] == 'o'):
        return True
    else:
        return False


def intro():
    intro_board = [['1', '4', '7'], ['2', '5', '8'], ['3', '6', '9']]
    print("Welcome to Tic Tac Toe")
    print("You can pick location by identifying the position on the board. (There are 9 positions)")
    print("The player who plays first will be using 'x' and the second player will be using 'o'.")
    for y in range(3):
        print(add_sep('|', [intro_board[x][y] for x in range(3)]))


def main():
    game_board = [[' '] * 3 for _ in range(3)]
    taken_positions = []
    symbol = 'x'
    num_moves = 0
    win = False
    intro()
    while num_moves < 9 and not win:
        position = get_position()
        taken = check_position(position, taken_positions)
        if taken:
            print("Position taken! Try again.")
        else:
            draw_position(position, symbol, game_board)
            taken_positions.append(position)
            symbol = change_symbol(symbol)
            num_moves = num_moves + 1

        win = check_win(game_board)
        if win:
            print("We have a winner!")
            break

    if num_moves == 9 and not win:
        print("WOW! You guys are good! DRAW!!!")


# MAIN
main()
print("Thanks for playing! Exiting")
\$\endgroup\$
4
\$\begingroup\$

magic numbers

I see the value 3 (and 9) in your code when it refers to the board size. It is best to prevent such magic number, and either replace them with variables, or with constants

BOARD_SIZE = 3

num_to_coord

you can use divmod to calculate the quotient and remainder of a modulo division in 1 step, so num_to_coord gets reduced to:

def num_to_coord(num):
    """calculates the coordinate of a 1-indexed position number

    Examples
    --------
    >>> num_to_coord(1)
    (0, 0)
    >>> num_to_coord(3)
    (0, 2)
    >>> num_to_coord(4)
    (1, 0)
    """
    return divmod(num - 1, BOARD_SIZE)

The effect is a 90° rotation of the board

multi-line strings

Python has multiline string literals, so your welcome ca

def intro():
    welcome_message = """
    Welcome to Tic Tac Toe
    ______________________
    You can pick location by identifying the position on the board. (There are 9 positions)
    The player who plays first will be using 'x' and the second player will be using 'o'.
    """
    print(welcome_message)
    intro_board = """
    |1|2|3|
    |4|5|6|
    |7|8|9|"""
    print(intro_board)

State

to separate state and representation, I would use an enum.Enum to keep the state of a board position

import enum
class Position(enum.Enum):
    EMPTY = " "
    PLAYER_1 = "x"
    PLAYER_2 = "o"

then later on, you can define the game_board as game_board = [[Position.EMPTY] * BOARD_SIZE for _ in range(BOARD_SIZE)]

This allows you to check whether there are still open positions in this way:

def check_board_open(the_game_board):
    """checks whether there are still open positions in the board"""
    return any(Position.EMPTY in row for row in the_game_board)

instead of having to keep a count of how many moves have been done

and drawing the board becomes:

def format_row(row, separator="|"):
    return (
        separator
        + separator.join(position.value for position in row)
        + separator
    )


def draw_board(the_game_board, separator="|"):
    return "\n".join(format_row(row, separator) for row in the_game_board)

In your code, you also put changed the board in the method to draw the board, which is a serious violation of the separation of concerns.

taken_positions

you define taken_positions as a list, but the order is unimportant, and the main goal is check for containment. A set is a more suited collection for this purpose.

check_position

def check_position(the_position, the_taken_positions):
    if the_position in the_taken_positions:
        return True
    if the_position not in the_taken_positions:
        return False

can be simplified to

def check_position(the_position, the_taken_positions):
    return the_position in the_taken_positions

and then dropped altogether to make it inline

user input

You trust your user to input a valid number. This can be made more robust, and integrate the check whether the position is empty

class GameEnd(Exception):
    pass


def get_empty_position(the_game_board):
    max_position = BOARD_SIZE ** 2
    while True:
        user_input = input(f"Enter a position [1-{max_position}]: (q to quit)")
        if user_input.lower() == "q":
            raise GameEnd()
        try:
            position = int(user_input)
            if not 0 < position <= max_position:
                continue
            x, y = num_to_coord(position)
            if the_game_board[x][y] != Position.EMPTY:
                print("Position taken! Try again.")
                continue
            return x, y
        except ValueError:
            pass

This keeps asking for input until a valid, empty position is given, or "q". If the user wants to end the game, this raises a GameEnd exception.

You also don't need the taken_positions, since checking whether the position is taken is done here immediately, and directly compared to the game board instead of a second data structure.

Win situations

@feelsbadman is correct that you can decouple the different rows, columns and diagonals to check for a win condition, but I think the way he implements it can be improved. For tips on looping, check the great talk: "Looping like a Pro" (video on Youtube)

Instead of looping over the indices, you can loop over the rows or columns, and then see whether there is a sole player in that row or column.

To see whether there is a winner in a row, columns or diagonal, you can use this:

def get_winner(row):
    row_set = set(row)
    if len(row_set) == 1 and Position.EMPTY not in row_set:
        return row_set.pop()

This returns the winner if there is one, or None if there isn't

def check_win_horizontal(the_game_board):
    for row in the_game_board:
        winner = get_winner(row)
        if winner:
            return winner
    return None

to check vertical, you can transpose the board with zip(*the_game_board) so a separate vertical method is unnecessary.

For the diagonal, you can define 2 diagonals:

diagonal1 = {the_game_board[i][i] for i in range(BOARD_SIZE)}
diagonal2 = {the_game_board[i][-(i + 1)] for i in range(BOARD_SIZE)}

and then check them like this:

def check_win_diagonal(the_game_board):
    diagonal1 = {the_game_board[i][i] for i in range(BOARD_SIZE)}
    diagonal2 = {the_game_board[i][-(i + 1)] for i in range(BOARD_SIZE)}
    winner = get_winner(diagonal1) or get_winner(diagonal2)
    return winner

The general method to check whether there is a winner then simply is:

def check_win(the_game_board):
    return (
        check_win_horizontal(the_game_board)
        or check_win_horizontal(zip(*the_game_board))
        or check_win_diagonal(the_game_board)
    )

change_symbol

can be simplified to

def change_symbol(the_symbol):
    symbols = {"x": "o", "o": "x"}
    return symbols[the_symbol]

But this can also be done differently:

In the main method, you define the players as a itertools.cycle of (Position.PLAYER_1, Position.PLAYER_2). To get the next player, you just do player = next(players)

changing the board is then as simple as

    x, y = get_empty_position(game_board)
    game_board[x][y] = player 

main

def main():
    game_board = [[Position.EMPTY] * BOARD_SIZE for _ in range(BOARD_SIZE)]
    players = cycle((Position.PLAYER_1, Position.PLAYER_2))
    intro()
    while check_board_open(game_board):
        player = next(players)
        print(f"Player {player.value} to move")
        x, y = get_empty_position(game_board)
        game_board[x][y] = player
        print(draw_board(game_board))
        winner = check_win(game_board)
        if winner:
            return winner

main guard

Best put the code that should be run as a script behind a if __name__ == "__main__": guard, so you can import this file with a minimum of side effects

def ask_retry():
    user_input = input("press ENTER to continue, q, then ENTER to quit")
    return not user_input.lower() in {"q", "quit"}

if __name__ == "__main__":
    retry = True
    while retry:
        try:
            winner = main()
            message =  f"{winner.name} won!" if winner else "WOW! You guys are good! DRAW!!!"
            print(message)
            retry = ask_retry()
        except GameEnd:
            print("Thanks for playing! Exiting")
            retry = False

testing

By separating the methods like this, you can easily test them with unit tests. For code like this, which is 140 lines long, you can easily test it by hand, but if you want to incorporate changes later, like varying board sizes, a working, complete test suite will be a great help in spotting bugs.

\$\endgroup\$
  • \$\begingroup\$ Great stuff, going to take me a while to understand all this. \$\endgroup\$ – BobPage Jan 9 at 21:02
2
\$\begingroup\$

You could begin with win checking. The way you have done it works fine. But what if you were to resize the board? lets say 5x5? You would need to write lots of if/else statements to check for winning cases.

Now, instead of using loads of if/else statements to look for winning cases you could use for loops to look trough rows, columns, and crosses as shown here:

enter image description here

Here is an example of what I am talking about (you can use this if you wish so):

this one is for column checking (top-down)

# check columns
# symbol can be x or o
def checkCol(symbol, board):
    counter = 0
    for i in range(len(board)):
        for j in range(len(board)):
            if board[j][i] == symbol:
                counter += 1
            else:
                counter = 0
        if counter == len(board):
            break
    return True if counter == len(board) else False

and this is for row checking (left-right)

# check rows
# symbol can be x or o
def checkRow(symbol, board):
    counter = 0
    for i in range(len(board)):
        for j in range(len(board)):
            if board[i][j] == symbol:
                counter += 1
            else:
                counter = 0
        if counter == len(board):
            break
    return True if counter == len(board) else False

Positive cross:

#prositive cross (top left to bottom right)
def checkCrossPositive(symbol, board):
    counter = 0
    for i in range(len(board)):
        if board[i][i] == symbol:
            counter+=1
        else:
            counter = 0
    return True if counter == len(board) else False

Negative cross:

# negative cross (top right to bottom left)
def checkCrossNegative(symbol, board):
    counter = 0
    j = len(board)-1
    for i in range(len(board)):
        if board[i][j] == symbol:
            counter += 1
        else:
            counter = 0
        j-=1
    return True if counter == len(board) else False

Here's how it can be used:

def check_win(board):
    return \
        (checkRow('x', board) or checkRow('o', board) )\
    or  (checkCol('x', board) or checkCol('o', board))\
    or  (checkCrossPositive('x', board) or checkCrossPositive('o', board))\
    or  (checkCrossNegative('x', board) or checkCrossNegative('o', board))

EDIT:

Avoid manually placing numbers here and there such as in

if num_moves == 9 and not win:

instead of that you could have it in a "constant" variable. (I know python doesn't have constant but just for the sake of best-practice)

if num_moves == MAX_MOVES and not win:

EDIT (Explanation for column checker):

Part 1

Usually when we iterate over a two dimensional array we use i and j as X and Y where i=Y and j=X let's say we have a two dimensional array as this:

array = [
    [1, 2, 3, 4, 5],
    [6, 7, 8, 9, 0]
]

If we iterate over this array the usual way we'd get 1,2,3,4,5 & 6,7,8,9,0 as output because the Y-axis ( i ) represents indexes of inner arrays (those that contain numbers 1,2,3 ...). First (inner-)array has index 0, the second one has 1 and so on it goes.

This way we are iterating row => col1, col2, col3, ... meaning i => j1, j2, j3, ... but since we need to look for columns rather than rows we need to switch the usage of i and j where, for example, to access number 1 in the first array we would have the following: array[i][j] or array[0][0] but in this case we have to use it as array[j][i] (<- notice that i switched them).

Now when we have switched the index (representors?) we would get an output as this: 1,6 & 2,7 & 3,8 & 4,9 & 5,6

Here is a "GIF" to animate the concept behind this: https://i.imgur.com/4HqLpcD.mp4

Part 2

As you might have noticed there is a counter variable initialized with 0 at the very beginning of the code. The purpose of this counter is to keep track of how many of _symbol_ we have found while iterating of columns.

Inside the second loop there's a code block:

if board[j][i] == symbol:
    counter += 1
else:
    counter = 0

The purpose of this part is to increase our counter variable by 1 if a symbol is found. if not it is re-set back to 0. This is because for someone to win, the whole column would have to be of the same symbol, let's say symbol o and if any section in the column does not match our symbol o then it means player-o can not win because in order for him to win all of the sections in the current column (index 0 to 4) would have to be of the same symbol.

When the first column has been scanned, we must check whether we have found enough symbols to call it a win or not.

if counter == len(board):
    break

Since our board is a square we can safely compare our counter to the length of our board. Why? Because there are as many indexes(0 to 4) as the length(5) of our board. So when I have filled the first column with symbol-o there will be exactly 5 os in that column.

When the above statement is true; main loop will break and then a boolean value is returned:

return True if counter == len(board) else False

As obvious as it is: When counter is equal to the length of our board True(won!) is returned otherwise False (didn't win)

\$\endgroup\$
  • \$\begingroup\$ Thank you for the advice on how to loop through the check_win. I have to admit though, looking at the code, I'm pretty confused. Can you explain the code in a bit more detail? If not, no problem. \$\endgroup\$ – BobPage Jan 8 at 18:48
  • \$\begingroup\$ @BobPage UPDATE: Added explanation for column checking (will add other explanations as soon as possible) btw, let me know if you have trouble understanding anything \$\endgroup\$ – Feelsbadman Jan 9 at 1:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.