9
\$\begingroup\$

I wrote a simple tic-tac-toe game in Python. I'm pretty much a novice, but I do have some experience with coding other things in Python. I just wanted to see if I was capable of writing a simple tic-tac-toe game, since I never really went through that rite of passage. I realized it wasn't that difficult for me, but I think I'm lacking knowledge to take me to the next level. My code utilizes a lot of if-else statements and I feel like I could do a better job with using functions. Here is the code:

table = {
    "a1": " ",
    "a3": " ",
    "a2": " ",
    "b1": " ",
    "b2": " ",
    "b3": " ",
    "c1": " ",
    "c2": " ",
    "c3": " ",
}

def check_win(letter):
    if table["a1"] == letter and table["a2"] == letter and table["a3"] == letter:
        print(f"Player {letter} wins")
        return False
    elif table["b1"] == letter and table["b2"] == letter and table["b3"] == letter:
        print(f"Player {letter} wins")
        return False
    elif table["c1"] == letter and table["c2"] == letter and table["c3"] == letter:
        print(f"Player {letter} wins")
        return False
    elif table["c1"] == letter and table["b2"] == letter and table["a3"] == letter:
        print(f"Player {letter} wins")
        return False
    elif table["c3"] == letter and table["b2"] == letter and table["a1"] == letter:
        print(f"Player {letter} wins")
        return False
    elif table["a1"] == letter and table["b1"] == letter and table["c1"] == letter:
        print(f"Player {letter} wins")
        return False
    elif table["a2"] == letter and table["b2"] == letter and table["c2"] == letter:
        print(f"Player {letter} wins")
        return False
    elif table["a3"] == letter and table["b3"] == letter and table["c3"] == letter:
        print(f"Player {letter} wins")
        return False
    else:
        return True

game_on = True
display_table = f"""
        1   2   3 
    a | {table["a1"]} | {table["a2"]} | {table["a3"]} |
    b | {table["b1"]} | {table["b2"]} | {table["b3"]} |
    c | {table["c1"]} | {table["c2"]} | {table["c3"]} |
    
    """
print(display_table)

count = 0
while game_on:
    x_location = input("Player X: Pick a row and column (e.g. a1, a2, etc.): ").lower()
    check_x = True
    while check_x:
        if table[x_location] == " ":
            table[x_location] = "X"
            count += 1
            check_x = False
        else:
            x_location = input("That is an invalid spot. Please try again: ").lower()
            check_x = True

    display_table = f"""
        1   2   3 
    a | {table["a1"]} | {table["a2"]} | {table["a3"]} |
    b | {table["b1"]} | {table["b2"]} | {table["b3"]} |
    c | {table["c1"]} | {table["c2"]} | {table["c3"]} |

    """

    print(display_table)

    game_on = check_win("X")
    if game_on == False:
        break

    if count >= 9:
        print("There is no winner")
        break

    o_location = input("Player O: Pick a row and column (e.g. a1, a2, etc.): ")
    check_o = True
    while check_o:
        if table[o_location] == " ":
            table[o_location] = "O"
            count += 1
            check_o = False
        else:
            y_location = input("That is an invalid spot. Please try again: ").lower()
            check_o = True   

    display_table = f"""
        1   2   3 
    a | {table["a1"]} | {table["a2"]} | {table["a3"]} |
    b | {table["b1"]} | {table["b2"]} | {table["b3"]} |
    c | {table["c1"]} | {table["c2"]} | {table["c3"]} |

    """
    print(display_table)

    game_on = check_win("O")
    if game_on == False:
        break

I'm trying to figure out a way where I can simplify the code in the check_win function. What can I do instead of writing so many lines of different if/elif statements?

I also think I could put the recurring display_table and print(display_table) within another function, but I'm not sure how I could easily update the actual table since it would be local variable within a function.

Any other suggestions for improvement are appreciated.

\$\endgroup\$
1
  • \$\begingroup\$ While it may be a bit unrelated, I like to give check_win the coordinates of the last piece played, so you only have to check one line, one column, and optionaly the diagonals. It's easier to read \$\endgroup\$
    – Sho
    May 8 at 15:52

3 Answers 3

11
\$\begingroup\$

Put all code in functions. This is what experienced programmers do, so you would be wise to adopt the practice even if you don't fully appreciate all of the reasons yet. After that change, you'll end up with a program having the following structure. Note that this change requires us to pass the table into check_win() as an argument, but that's an easy and reasonable change.

def main():
    table = {...}
    ...

def check_win(table, letter):
    ...

if __name__ == '__main__':
    main()
    

Use functions to eliminate duplication: displaying the table. You build a display table in several places. Put the code in one spot: inside a function.

# The function.
def display_table(table):
    return f"""
        1   2   3 
    a | {table["a1"]} | {table["a2"]} | {table["a3"]} |
    b | {table["b1"]} | {table["b2"]} | {table["b3"]} |
    c | {table["c1"]} | {table["c2"]} | {table["c3"]} |

    """

# A usage example:
print(display_table(table))

Use functions to eliminate duplication: handling different players. The main() functions is basically two large chunks of duplicate code, one for X and one for O. That means every code edit has to be make in two places, which is annoying and error-prone. Put the behavior in a function, passing in any needed parameters as arguments. If we just make that change (and remove the unnecessary check variable), we get this:

def make_play(table, letter):
    location = input(f"Player {letter}: Pick a row and column (e.g. a1, a2, etc.): ").lower()
    while True:
        if table[location] == " ":     # What should you do if location not in table?
            table[location] = letter
            break
        else:
            location = input("That is an invalid spot. Please try again: ").lower()
    print(display_table(table))

Use a variable to eliminate duplication: tightening up the main logic further. After that change, you'll notice some more annoying duplication in main(). In this case, we can deal with the situation by having a player variable and just toggling back and forth between the players. Again, the boolean flag isn't really helping much so we can drop it. Note that this method of toggling the players is fairly crude, but you'll learn about nicer ways as you go forward with Python. Also note that the count variable is unneeded: just write a function to take the table and return true or false based on the whether the dict contains any spaces.

def main():
    ...

    count = 0
    player = 'X'
    while True:
        make_play(table, player)
        count += 1
        if not check_win(table, player):
            break
        elif count >= 9:                # Not needed. Write a function.
            print("There is no winner")
            break
        else:
            player = 'O' if player == 'X' else 'X'

Use boolean logic or a data structure to eliminate duplication. If you are following the logic so far, the check_win() function will seem super annoying at this point: it repeats the same print and return statements eight times. One approach is to combine the checks into a single logical test: that at least eliminates the duplicate print and return statements. Even better is to define a data structure to define the checks. While you're considering such changes, you might want to alter check_win() to return true if there's a winner, which makes more sense given the other edits proposed so far. Also, the printing would be better done in main() than in the win checking code.

# Boolean combination.

def check_win(table, letter):
    won = (
        (table["a1"] == letter and table["a2"] == letter and table["a3"] == letter) or
        (table["b1"] == letter and table["b2"] == letter and table["b3"] == letter) or
        etc...
    )
    if won:
        print(f"Player {letter} wins")
        return False
    else:
        return True

# A data structure to drive the logic.

CELL_GROUPS = (
    ("a1", "a2", "a3"),
    ("b1", "b2", "b3"),
    ...
)

def check_win(table, letter):
    for cg in CELL_GROUPS:
        if all(table[cell] == letter for cell in cg):
            print(f"Player {letter} wins")              # Do this in main()
            return False                                # Reverse.
    return True                                         # Reverse.
\$\endgroup\$
2
  • \$\begingroup\$ This was great. Thank you so much. I feel like more beginner tutorials and guides should emphasize what you discussed here. \$\endgroup\$
    – jp207
    May 7 at 19:01
  • 1
    \$\begingroup\$ @jp207 You are very welcome, and I agree! Functions and data structures are the key to programming, but a large share of introductory material on coding focuses on what I would, perhaps uncharitably, describe as an obsession with interactivity – endless sequences of print() and input() statements – plus some if-then logic and looping thrown in just to make it look like code. OK, rant over. Good luck with your Python learning. \$\endgroup\$
    – FMc
    May 7 at 19:16
5
\$\begingroup\$

General Style

Add a main guard

This convention is specific to Pyton.

Use a block if __name__ == '__main__': to put all your main logic into.

Functions should do one thing

Have check_win return True or False, nothing else.

Pull logic into functions

Try to factor any logic you can easily name into a function.

  • "display the game board" becomes display(table)
  • "get the next move" becomes get_move(table, player) (include the error checking!)

Global varables

Avoid global variables such as table, which are universally considered bad style and hard to debug. I'm afraid this one will be harder to understand why for small programs, but trust me that you will appreciate habits you build now later on.

Avoid human thinking when the computer can check

Program defensively.

This the principle that makes you write if count >= 9 for tie games, rather than if count == 9 -- it's more "defensive", or idiot-proof. We all make mistakes sometimes. If you write things in a defensive way, making one mistake will be caught by another part of the program.

You carefully reason about when a player can win. It's less error-prone to simply check the entire board state. Change check_win into calculate_winner. It should return something like X, O, tie, or ongoing game

Repetition

Your instincts are good. Any time you see repetition it's good to think about how to remove it. Making everything shorter makes it easier to read and write. Having logic in one place instead of several, means it's guaranteed to be consistent (less errors) and easy to update (only change one place).

The main loop

Under the section while game_on:, you do one thing for X, and then another thing for O. 99% of this logic is the same with X replaced by O. Here's an example of how to only have one copy of the logic.

player_won = False
while not player_won:
  for player in ("X", "O"):
      player_turn(player)      
      player_won = check_win(player)

def player_turn(player):
    player_location = input(f"Player {player}: Pick a row and column (e.g. a1, a2, etc.): ").lower()
    check_input = True
    while check_input:
        if table[player_location] == " ":
            table[player_location] = player
            count += 1
            check_player = False
        else:
            player_location = input("That is an invalid spot. Please try again: ").lower()
            check_player = True

    display_table = f"""
        1   2   3 
    a | {table["a1"]} | {table["a2"]} | {table["a3"]} |
    b | {table["b1"]} | {table["b2"]} | {table["b3"]} |
    c | {table["c1"]} | {table["c2"]} | {table["c3"]} |
    
    """

    print(display_table)

Notice that I've split out the game logic into a function. This is not only good style, it also lets you call return early if you need.

check_win repetition

First, remove print(f"Player {letter} wins") everywhere. This can be printed based on the return value of the function. After that, you still have repetitive if-then statements.

One approach is to make a list of possible lines:

lines = [
         ("a1", "a2", "a3"), ("b1", "b2", "b3"), ("c1", "c2", "c3"), // Up-down
         ("a1", "b1", "c1"), ("a2", "b2", "c2"), ("a3", "b3", "c3"), // Left-right
         ("a1", "b2", "c3"), ("a3", "b2", "c1"), // Diagonal
        ]

def check_win(letter):
    for line in lines:
        if table[line[0]] == letter and table[line[1]] == letter and table[line[2]] == letter:
            return True
    return False

Another approach is to index the table as two numbers (table["a3"] becomes table[0][2]). Writing it this way, you can write:

def check_win(letter):
    for row in range(3): // Left-right
        if table[row][0] == table[row][1] == table[row][2] == letter:
            return True
    for col in range(3): // Up-down
        if table[0][col] == table[1][col] == table[2][col] == letter:
            return True
    if table[0][0] == table[1][1] == table[2][2] == letter: // Diagonal
        return True
    if table[2][0] == table[1][1] == table[0][2] == letter: // Diagonal
        return True

The index approach also allows you to display the table with a nested loop, as well as initialize the table with a nested loop. Overall if you had to pick one, a two-index array is better.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks. This was helpful. Can you give me an example of what one should do instead of using table as a global variable? Should I put the table in a function and access it accordingly? \$\endgroup\$
    – jp207
    May 7 at 19:02
  • \$\begingroup\$ Pass table into functions, as in check_win(table, player) \$\endgroup\$ May 7 at 19:40
3
\$\begingroup\$

You definitely need more functions.

You state that you could put print(display_table) in a function, but not sure how you could update the table since it would be a local variable within the function. Yet, you have check_win(letter), which is a function which accesses the global variable table. How are you able to do one but not the other?

At the top, you layout table in rows in a strange order, with "a3" between "a1" and "a2". Dictionaries maintain the order of their keys, so this artifact will persist. You could make the table look more like its logical structure by defining table as:

table = {
    "a1": " ", "a2": " ", "a3": " ",
    "b1": " ", "b2": " ", "b3": " ",
    "c1": " ", "c2": " ", "c3": " ",
}

You might want to have a function which creates the table, so you can play the game more than once. Perhaps something like...

CELLS = {"a1", "a2", "a3", "b1", "b2", "b3", "c1", "c2", "c3"}

def create_game_grid():
    return {cell: " " for cell in CELLS}

table = create_game_grid()

Your game crashes if the player enters "x3" instead of "c3", with a KeyError. You are checking that the player is entering an unplayed spot, but not a valid spot! Leveraging the CELLS set above, you could use:

        if x_location in CELLS and table[x_location] == " ":
            ...
        else:
            ...

There is no need for a check_x variable. Just break out of a while True loop, eliminating 2 unnecessary lines:

    while True:
        if x_location in CELLS and table[x_location] == " ":
            table[x_location] = "X"
            count += 1
            break
        else:
            x_location = input("That is an invalid spot. Please try again: ").lower()

Ditto for the O player ... or just have the code once as suggested in Fmc's answer.

check_win() returns True for no win, and False if there is a win. Confusing. Come up with a better name, or reverse the return values.

Use named CONSTANTS. If you accidentally wrote check_win("0") or check_win("o"), you might eventually discover the O player can never win. Where as if you defined constants at the top of the program ...

EMPTY = " "
PLAYER_X = "X"
PLAYER_O = "O"

... then if you accidentally typed in a name that didn't exist, like check_win(PLAYERO), your program would immediately fail with a NameError. Your IDE probably can help you with autocompletion of variable/function names, so being verbose is not a burden. You could also easily change the game to use different symbols, by just changing two lines at the top:

PLAYER_X = "❌"
PLAYER_O = "⭕"
\$\endgroup\$

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.