If Statement Error (Tic Tac Toe)

[Jim Segrave]

In article <1130878530.389205.179640 at g43g2000cwa.googlegroups.com>,
 <ale.of.ginger at gmail.com> wrote:
>Greetings!  I am trying to make a multiplayer (no AI, 2 person) game of
>tic tac toe in Python.  So far it has been pretty simple.  My only
>concern is with the win checking to see if a person has won.  At first
>it looked like it was working, but now it sometimes assigns a win when
>you enter an X or O (doesn't matter) on certain tiles (row 1, column 1
>won't be an error, but row 2, column 3 will be...).  If you can find
>the problem, I'd be very thankful!  Here's the code:

I see one of the reponses directs you to a web site about how to ask
questions and expect answers - there's some very good advice
there. Nonetheless, I'll give you the benefit of the doubt and assume
that you are _very_ new to programming. I hope I haven't redone your
homework for you, but if I have, I would suggest you don't submit my
answer, I think it might be recognised as not being your own.

The biggest problem with asking someone for help with a program like
this is that it's very large (not in and of itself a problem) and,
even to the most rapid glance, the program contains huge swathes of
essentially the same code repeated over and over with only tiny
changes. When I see that, all the warning bells start ringing.

My first hint would be that anytime you find yourself typing exactly
the same (or almost exactly the same thing) over and over, you are
probably doing something wrong. In your original code, you have a
whole section for handling player X, then you have virtually identical
code, for player O. 

You should almost never need to do this. There are a couple of easy
ways to save retyping the same code:

1) create a function. The function body will have all the identical
   code, the function arguments will indicate what's different between
   the two sections. I think this is a hint from Kernighan and Pike
   from a book dating back to the late 1960's - subroutines are used
   to show what code has in common, the arguments are used to show
   what's different.

or

2) put the duplicated code inside a loop and let the loop variable be
   used to make the small changes. That's what I did in the hasty
   rework of your program below.

Then, when we look at the code for a single player, you have 9
separate tests to see if a cell is occupied:

    if row == 2 and column == 1:
        if gameboard[3] != ('O' or 'X'):
            gameboard[3] = ('O')
        else:
            print "This cell is already filled."
        turnnumber = turnnumber - 1

For each row and column, you know which cell to look at, so why have
tests for every possible input, when only one test will apply for any
given user input? 

The cell you want is:
    gameboard[3 * (row - 1 ) + (column - 1)]
so one test can replace 9. In reworking this, I calculate this
location once and save it as 'loc', since I'll be needing the location
later.

Then there's the question of the test if the cell is occupied: 

    if gameboard[3] != ('O' or 'X'):

This doesn't test if the cell contains an 'O' or an 'X', what it
actually tests is whether the cell contains an 'O'. The reason for
this is that Python sees the 'or' operator and tests if the left hand
side is True, which, since it is a string, tests if the string is
empty, which it isn't. Python, when given EXPR or EXPR returns the
left hand EXPR if it is True, otherwise it returns the right hand
expression. Also, since you know that unoccupied cells contain a
space, why not test for the cell containing a space, instead of
testing that it doesn't contain an 'O' and it doesn't contain an 'X'?,
no 'or' operator required?  If you must use 'or', then you'd have to
write:

     if gameboard[3] == 'O' or gameboard[3] == 'X':
         print "This cell is already filled."

The next point is the entire user input section. You have a while loop
waiting until the game is won, which gets the user input at the top
and goes through the entire rest of the code on every user input. It's
clearer to handle the user input within a smaller loop which runs
forever (while 1:) prompting for input and which uses the 'break'
statement to drop out of the loop when the input is valid. If I were
writing this myself, I'd probably extract it into a function, simply
to separate it from the game playing code.

You display the current board with a very lengthy print statement,
naming each cell to be printed and surrounding it with individual
']''s. But this is the sort of job computers do well. What you
actually want to do is print three lines, one for board[0] through
board[2], another for board[3] through board[5] and the last for
board[6] through board[8]. 

   print "[%s][%s][%s]" % (board[0], board[1], board[2]) etc.

would work in three clearer lines, but we may as well let the computer
do the calculation of the indices using a for loop. A final pair of
improvements is to not type the [%s] 3 times, nor the name of the
board 3 times. We can get the format string using the "" * n notation:

   '[%s]' * 3

We can get the cells to print as a string slice:

    tuple(board[r : r + 3])

I leave it to you to find out why the tuple() call is needed.

When getting the input, you use the function input(), which sounds
reasonable, given the name, but in fact is not at all what you
want. This function gets a line of input from the user and evaluates
it, returning the result. You could enter '1+2' for the row and Python
would return 3. If you type an invalid expression or a letter, Python
will terminate your program, which seems a bit drastic. Instead, I
used raw_input, which simply gets a string, then checked if the string
was one of '1', '2', or '3', using the 'in' operator on the sequence
of valid values. While not the sort of validation you'd use in may
places, when the number of accptable inputs is this small, the
resulting code becomes instantly understandable. You can now type
almost anything except control-C and it will not stop the program.

After validating the input, you check that the cell is free, but,
since you don't have a loop to get the input, you have to reset the
player and drop to the bottom of the program just to go back to the
main while loop. Keeping all the validation in a single loop you can't
leave until you come up with a good location.

The final part (and the one you originally asked about) was the pair
of if statements you wrote to check if the game had been one. Once
again, you misunderstood the logical operators, 'and' in this
case. The result of the expression 

   (gameboard[3] and gameboard[4] and gameboard[5]) == 'O'

is not a test of whether the three cells all contain 'O', it is a test
of whether the first two cells contain a non-empty string _and_ the
last cell contains 'O'.

Naming the 8 possible ways to win at tictactoe in a single if
statement seems wrong. The statment borders on the unreadable.

Instead, consider what winning means: you have three identical cells
not containing spaces either horizontally ( in board[i], board[i + 1]
and board[i +2] for values of i of 0, 3, or 6. Another way of winning
is to have three identical non-space cells in a column = board[i],
board[i + 3], board[i + 6] for values of i of 0, 1, or 2. The third
way to win is to have a diagonal set of three identical non-space
cells = board[0], board[4], board[8] or board[2], board[4], board[6]

The first two types of wins cry out for a for loop to try the
horizontal or vertical wins, with a little simple work, the same for
loop over values of 0, 1, 2 can do both types. I'm too lazy to think
up a clever way of doing the two diagonals.

The actual test looks like:

  board[i] == board[i + 1] == board[i + 2] and board[i] != ' '

taking advantage of the way Python handles multiple '=='s strung out
together ( in effect a == b == c is treated as a == b and b == c ).

The resulting rework of the program follows - it's by no means optimal
but it's 1/3 the length and, in my opinion, much more readable. 

It still suffers from having too many while loops depending on break
statements, but changing that would require me writing it over from
scratch.

# TIC TAC TOE
# Started: 10/31/05
# Ended:  still in progress

# There's no need for the variable 'loop'
while 1:
    print "TIC TAC TOE"
    print "1 - Play Multiplayer"
    print "2 - Quit"
    option = raw_input("> ")

    if option == '2':
        break   

    if option != '1':
        continue

    # MAIN GAME LOOP
    print "Rules:  You will alternate turns."
    print "On your turn, you can place your letter (O = Player 1 or X = Player 2)",
    print "in any unoccupied square."
    print "The first to get 3 in a row wins.  Good luck!"

    board = [' ',' ',' ',' ',' ',' ',' ',' ',' ']

    turns = 0
    player = 0
    win = 0

    # loop until the board is full
    while turns < 9:
        # loop until we have valid input
        while 1 :
            print " "
            print "Player %d" % (player + 1)
            print " "
            
            for r in range(0, 9, 3):
                print "[%s]" * 3 % tuple(board[r : r + 3]) 

            row = raw_input("Row: ")
            column = raw_input("Column: ")
                
            if row not in ['1', '2', '3' ] or \
                   column not in ['1', '2', '3']:
                print "Invalid selection, please try again"
                continue

            # convert row and column to array index
            loc = 3 * (int(row) - 1) + int(column) - 1
            if board[loc] == ' ':
                break
                
            print "This cell is already filled."

        # now we have a valid location, fill it in
        board[loc] = ['X', 'O'][player]
            
        for i in range(3):
            # check for horizontal win
            if board[3 * i] == board[ 3 * i + 1] == board[3 * i + 2] and \
                   board[3 * i] != ' ':
                win = 1
                break
            if board[i] == board[i + 3] == board[i + 6] and \
                   board[i] != ' ':
                win = 1
                break
        else:
            # no horizontal or vertical win yet
            # test the diagonals
            if (board[0] == board[4] == board[8] or 
                board[2] == board[4] == board[6]) and board[4] != ' ':
                win = 1

        if win:
            break
            
        # if we reach here, the game isn't over,  flip players
        turns += 1
        player = (player + 1) % 2

    # the game has finished
    if win:
        print "\nPlayer %d wins!" % player
        for r in range(0, 9, 3):
            print "[%s]" * 3 % tuple (board[r : r + 3])
            print
    else:
        print "Tie"
            


-- 
Jim Segrave           (jes at jes-2.demon.nl)