2
\$\begingroup\$

I would really appreciate a general review of my Tic Tac Toe game in Python as I have spent a lot of time on it. Any improvements or suggestions?

def print_board(board):
    print "The board look like this: \n"
    for i in range(3):
        print " ",
        for j in range(3):
            if board[i*3+j] == 1:
                print 'X',
            elif board[i*3+j] == 0:
                print 'O',  
            elif board[i*3+j] != -1:
                print board[i*3+j]-1,
            else:
                print ' ',

            if j != 2:
                print " | ",
        print

        if i != 2:
            print "-----------------"
        else: 
            print 

def print_instruction():
    print "Please use the following cell numbers to make your move"
    print_board([2,3,4,5,6,7,8,9,10])


def get_input(turn):

    valid = False
    while not valid:
        try:
            user = raw_input("Where would you like to place " + turn + " (1-9)? ")
            user = int(user)
            if user >= 1 and user <= 9:
                return user-1
            else:
                print "That is not a valid move! Please try again.\n"
                print_instruction()
        except Exception as e:
            print user + " is not a valid move! Please try again.\n"

def check_win(board):
    win_cond = ((1,2,3),(4,5,6),(7,8,9),(1,4,7),(2,5,8),(3,6,9),(1,5,9),(3,5,7))
    for each in win_cond:
        try:
            if board[each[0]-1] == board[each[1]-1] and board[each[1]-1] == board[each[2]-1]:
                return board[each[0]-1]
        except:
            pass
    return -1

def quit_game(board,msg):
    print_board(board)
    print msg
    quit()

def main():

    # setup game
    # alternate turns
    # check if win or end
    # quit and show the board

    print_instruction()

    board = []
    for i in range(9):
        board.append(-1)

    win = False
    move = 0
    while not win:

        # print board
        print_board(board)
        print "Turn number " + str(move+1)
        if move % 2 == 0:
            turn = 'X'
        else:
            turn = 'O'

        # get user input
        user = get_input(turn)
        while board[user] != -1:
            print "Invalid move! Cell already taken. Please try again.\n"
            user = get_input(turn)
        board[user] = 1 if turn == 'X' else 0

        # advance move and check for end game
        move += 1
        if move > 4:
            winner = check_win(board)
            if winner != -1:
                out = "The winner is " 
                out += "X" if winner == 1 else "O" 
                out += " :)"
                quit_game(board,out)
            elif move == 9:
                quit_game(board,"No winner :(")

if __name__ == "__main__":
    main()
\$\endgroup\$
1
\$\begingroup\$

A few quick comments, in no particular order:


Python 3 uses print as a function, not a statement

You’ve tagged the question with [python-3.x], but the code uses print statements, not functions. If I try to run the code with Python 3, I immediately get an error:

  File "tictactoe.py", line 2
    print "The board look like this: \n"
                                       ^
SyntaxError: Missing parentheses in call to 'print'

Henceforth I’ll assume that you’re actually using Python 2.x, and the question was just mistagged.


Don’t catch overly broad exceptions.

In your get_input() function, you have except Exception as e, to catch the case where the user enters something that isn’t an integer. You can improve this in two ways:

  • Only catch the exception you‘re expecting to throw. If you catch plain Exception, you risk losing information about unexpected errors.

    In this case, you might get a ValueError if the user gives a string which can’t be coerced to an integer, so catch ValueError’s instead.

  • Limit the amount of code in the try block. The only line you expect to throw an exception is user = int(user), but you’ll catch exceptions thrown in any of the surrounding lines. Better to only include that line in the try block – again, you won’t lose information about unexpected errors.

Here’s how you can rewrite the body of the get_input() function accordingly:

user = raw_input("Where would you like to place " + turn + " (1-9)?
try:
    user = int(user)
except ValueError:
    print user + " is not a valid move! Please try again.\n"
    continue
if 1 <= user <= 9:
    return user-1
else:
    print "That is not a valid move! Please try again.\n"
    print_instruction()

That said, I was unable to find any invalid input that caused the script to crash – kudos.

Looking further, I see something worse in check_win():

try:
    if board[each[0]-1] == board[each[1]-1] and board[each[1]-1] == board[each[2]-1]:
        return board[each[0]-1]
except:
    pass

You should never do except: pass. This will (silently) swallow every form of exception – including SystemExit and KeyboardInterrupt, which you almost certainly don’t want to catch. In this case, you probably want to catch KeyError instead.


Get rid of the valid variable in get_input()

Its value is never actually used – it just serves to keep the loop going perpetually until the function returns. Better to use while True than track an unused variable.

Likewise for win in the main function.


Add comments and docstrings

There’s very little in the way of commentary in this code. That makes it hard to follow, and it will make things difficult when you come to maintain it in future.

Comments will help understand why you’ve written the code this way – for example, in get_input(), why do we decrement 1 from the value provided by the user? And docstrings are important if you want your code to be reusable.

\$\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.