3
\$\begingroup\$
def draw_board(board):
    display_board = ''

    for number in list(range(len(board))):
        if (number + 1) % 3 == 0:
            if number < 6:
                display_board += '  ' + str(board[number]) + '  \n—————————————————\n'
            else:
                display_board += '  ' + str(board[number]) + '  '
        else:
            display_board += '  ' + str(board[number]) + '  |'

    return display_board


def check_win(board, letter):
    if all(item == letter for item in board[:3]):
        return True
    elif all(item == letter for item in board[3:6]):
        return True
    elif all(item == letter for item in board[6:9]):
        return True
    elif all(item == letter for item in board[::3]):
        return True
    elif all(item == letter for item in board[1::3]):
        return True
    elif all(item == letter for item in board[2::3]):
        return True
    elif all(item == letter for item in board[::4]):
        return True
    elif all(item == letter for item in board[2:8:2]):
        return True


x = input('Who is X?  > ')
o = input('Who is O?  > ')

squares = {
    'nw': 0, 'n': 1, 'ne': 2,
    'w': 3, 'c': 4, 'e': 5,
    'sw': 6, 's': 7, 'se': 8
}

directions = list(squares.keys())

tic_tac_toe_board = [
    '.', '.', '.',
    '.', '.', '.',
    '.', '.', '.'
]

valid_turns = 0
x_turn = True
print(draw_board(tic_tac_toe_board) + '\n(Xs first) ', end='')

while valid_turns < 9:
    square = input('Exactly type in the following available directions: ' + ', '.join(directions) + '  > ')

    if square.lower() in directions:
        directions.remove(square.lower())

        if x_turn:
            tic_tac_toe_board[squares[square.lower()]] = 'X'
            x_turn = False
        else:
            tic_tac_toe_board[squares[square.lower()]] = 'O'
            x_turn = True

        valid_turns += 1
    else:
        print('INVALID MOVE')

    print(draw_board(tic_tac_toe_board))

    if check_win(tic_tac_toe_board, 'X'):
        print(x + ' has won!')
        break
    elif check_win(tic_tac_toe_board, 'O'):
        print(o + ' has won!')
        break

if valid_turns == 9:
    print('DRAW!')
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

Ways of improving/optimizing:

  • draw_board function.

    • prefer enumerate(board) over redundant list(range(len(board)))

    • all conditional branches within for loop have the same common expression to append ' ' + str(board[number]) + ' '. Thus, it can be moved up to reduce conditional branches

    • prefer flexible f-string formatting over awkward string concatenation combined with casting to str

    Now the optimized draw_board function would look as:

    def draw_board(board):
        display_board = ''
    
        for i, v in enumerate(board):
            display_board += f'  {v}  '
            if (i + 1) % 3 == 0:
                if i < 6:
                    display_board += '\n—————————————————\n'
            else:
                display_board += '|'
    
        return display_board
    
  • check_win function introduces many repetitive checks all(item == letter for item in board[...]) with the same returned value.
    A more flexible way is to create an additional function to check crossed row:

    def row_crossed(letter, row):
        return all(item == letter for item in row)
    

    Then, check_win would call any function on generator expression that yields all the needed sub-checks:

    def check_win(board, letter):
        return any((row_crossed(letter, board[:3]), row_crossed(letter, board[3:6]),
                row_crossed(letter, board[6:9]), row_crossed(letter, board[::3]),
                row_crossed(letter, board[1::3]), row_crossed(letter, board[2::3]),
                row_crossed(letter, board[::4]), row_crossed(letter, board[2:8:2])))
    
  • square dictionary. To reduce quotes mess it can be composed in concise way with zip and range functions:

    squares = dict(zip(('nw', 'n', 'ne', 'w', 'c', 'e', 'sw', 's', 'se'), range(9)))
    
  • tic_tac_toe_board easily replaced with tic_tac_toe_board = ['.'] * 9

  • square.lower() repeats 4 times within main while loop.
    Instead, extract that expression into a variable at once and refer it:

    square = input('Exactly type in the following available directions: {}  > '.format(', '.join(directions)))
    square = square.lower()
    
  • the whole condition:

    if x_turn:
        tic_tac_toe_board[squares[square.lower()]] = 'X'
        x_turn = False
    else:
        tic_tac_toe_board[squares[square.lower()]] = 'O'
        x_turn = True
    

    would be simplified as it relies on explicit negation of x_turn flag.
    Getting 2 lines against 6:

    tic_tac_toe_board[squares[square]] = 'X' if x_turn else 'O'
    x_turn = not x_turn
    
\$\endgroup\$
2
\$\begingroup\$

check_win

There are a lot of if and elif statements here. You can reduce this by utilizing python's any built in function. This will return True if any of the values passed in are True. And since you have a bunch of expressions that evaluate to boolean values, this can be rewritten like so:

def check_win(board, letter):
    return any([
            (item == letter for item in board[:3]),
            (item == letter for item in board[3:6]),
            (item == letter for item in board[6:9]),
            (item == letter for item in board[::3]),
            (item == letter for item in board[1::3]),
            (item == letter for item in board[2::3]),
            (item == letter for item in board[::4]),
            (item == letter for item in board[2:8:2])
        ])
    ])

Since all has to be passed an iterable, you can create a list containing the resulting Boolean values from these expressions. If any are True, then the function will return True.

This answer is the fastest on average (tested 1000 times).

Type Hinting

You should use type hints to make it clear what types of parameters are passed to functions, and what types are returned by functions. Lets take your check_win function for example:

from typing import List

def check_win(board: List[str], letter: str) -> bool:

This makes it clear that is accepts a list of strings as the board, as string representing the letter, and returns a boolean value.

Docstrings

Lets expand upon the check_win function. This can be even more descriptive by using a function docstring. This will allow you to put in words what the function is supposed to do. Take a look:

def check_win(board: List[str], letter: str) -> bool:
    """
    Determines if there is a winner in the passed board

    :param board -> List[str]: The playing board
    :param letter -> str: Letter (X/O) to check

    :return bool: True if there is a winner, False otherwise
    """

String Formatting

Instead of

print(x + ' has won!')

consider using an f"" string. This allows you to directly implement variables in your strings:

print(f"{x} has won!")

Additional Methods

Consider writing a couple more methods to reduce your open code in your program. By open I mean code that isn't contained within a function/class. Maybe a function like run_game that manages turns, and a display_winner that presents who won the game. I'll leave these up to you to implement.


All the above suggestions can be applied to each function within your code. I used check_win as an example for several, but they apply to both of your functions.

\$\endgroup\$
2
  • \$\begingroup\$ I don't agree with some of the suggestions on this answer. Type hints are not always necessary. Python didn't have them for decades. Docstrings are also another controversial topic. Usually you add them if there's nothing else you can do to make the code document itself (function name, variable names, tests, visual look of the code itself, etc.). Last, the code provided in this answer contains a lot of duplication. The list indices change, so why copy paste the whole logic with all? \$\endgroup\$ Commented Nov 17, 2019 at 10:20
  • \$\begingroup\$ @MikaeilOrfanian The type hints and docstring suggestions were suggestions. I guess I should have made it more clear that they were optional and not imperative to the functionality of the code. Yes, I agree that the check_win is repetitive, but it is faster than the original code. \$\endgroup\$
    – Linny
    Commented Nov 17, 2019 at 23:53

Your Answer

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

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