1
\$\begingroup\$

I am looking on how I can improve my writing style and/or how to simplify how I write my code.

# USER: TCG
# Tic Tac Toe (User vs Computer)

import random

def place(num,x):
    # Returns all the moves made
    for i in range(len(x)):
        if x[i] == num:
            pos = x.index(num)
            return x[(pos + 1)]
    return str(num)

def print_grid(move,player,x):

    pos_list.extend([move,player])

    # Grid on which the player and computer play on            
    grid = ["","-------------\n",
        "|",place(1,pos_list),"|",place(2,pos_list),"|",place(3,pos_list),"|\n",
        "|---+---+---|\n",
        "|",place(4,pos_list),"|",place(5,pos_list),"|",place(6,pos_list),"|\n",
        "|---+---+---|\n",
        "|",place(7,pos_list),"|",place(8,pos_list),"|",place(9,pos_list),"|\n",
        "-------------\n"]
    if x == 2:
        # Only prints one the player has made a move
        print '\n', ' '.join(grid)

def winner(x,player,xx):

    # Checks if there is a winner (Really messy, could do with code simplifying)
    if ((1 in x and 4 in x and 7 in x)or(1 in x and 2 in x and 3 in x)or(2 in x and 5 in x and 8 in x)or(3 in x and 6 in x and 9 in x)or
    (4 in x and 5 in x and 6 in x)or(7 in x and 8 in x and 9 in x)or(1 in x and 5 in x and 9 in x)or(3 in x and 5 in x and 7 in x)):
    # If prevents the A.I part from printing the statemnt
        if xx <> 1:
            print '\n'*5,"\'%s\'" %player, "HAS WON!"
        return 1 == 1

def computer_AI_part(listx):
    global computer_move

    # Chceks all possible values which the player can and enter to win and blocks it
    for x in range(1,10):
        if x not in pos_list:
            listx.append(x)
            if (winner(listx,'Computer',1)) == True:
                del listx[-1]
                computer_move = x
                return 1
            del listx[-1]

def computer_and_player():
    global computer_move,pos_list,player_list,computer_list
    replay,draw = 0,0

    while True:

        # Replay's the game
        if replay == 1:
            restart = raw_input("Would you like to replay?: ")
            if restart == "yes":
                pass
            else:
                return
        else:
            print "\nTic Tac Toe - Computer vs You", '\n'*2,"Computer goes first\n"

        replay,computer_move,players_move,loop_count,pos_list,player_list,computer_list = 0,0,0,0,[],[],[]

        for each in "XXXXX":
            loop_count += 1

            # Computer's Move
            if computer_AI_part(computer_list) or computer_AI_part(player_list) == 1:
                pass     
            else:
                while True:
                    computer_move = random.randint(1,9)
                    if computer_move in pos_list:
                        continue
                    break
            computer_list.append(computer_move)
            # Prints Grid
            print_grid(computer_move,'O',2)

            if loop_count == 5:
                if winner(player_list,'player',2) == True or winner(computer_list,'Computer',2) == True:
                    pass
                else:
                    print "Match Was a draw!"
                replay = 1
                break

            # Checks winner
            if winner(computer_list,'Computer',2) == True:
                replay = 1
                break

            # Player's Move
            while True:
                try:
                    players_move = int(raw_input("\n\'%s\' Enter a value from the grid to plot your move: " %each))
                    if players_move in pos_list or players_move < 1 or players_move > 9:
                        print "Enter an available number that's between 1-9"
                        continue
                    break
                except:
                    print "Enter a number"

            player_list.append(players_move)
            # Sets player's move for printing
            print_grid(players_move,each,1)

            # Checks winner again
            if winner(player_list,'player',1) == True:
                print_grid(players_move,each,2)
                winner(player_list,'player',2)
                replay = 1
                break

if __name__ == "__main__":
    computer_and_player()

Update: new version

\$\endgroup\$
3
\$\begingroup\$

Here's a section of my game:

'X' Enter a value from the grid to plot your move: exit
Enter a number

'X' Enter a value from the grid to plot your move: leave
Enter a number

'X' Enter a value from the grid to plot your move: quit
Enter a number

'X' Enter a value from the grid to plot your move: ^CEnter a number

I was just trying to do a short test, and then go back to the code and make some more changes, but I couldn't get out of the program without finishing the game. It's good that you don't allow just anything, but ^C at least should let me get out.


Your place() function could be a little better by using enumerate():

def place(num,x):
    # Returns the value in x at position num
    for i, v in enumerate(x):
        if v == num:
            return x[(i + 1)]
    return str(num)

Your naming could still be improved. How is one to know that x is a list of positions and values? There's more that I would change, but I'll get to that below.


You have a function called print_grid that makes a move, too. Either create a new function to make the move or rename the current function to show that it has side effects.


Global variables are almost never necessary. If you are tempted, you might be better off either passing them as arguments or defining a class. In this case, I think a class would be the better option.


pos_list would be better off being a dictionary (of course with a different name). It would be {pos: value, pos: value, ...} instead of [pos, value, pos, value, ...]. That way, lookups are easier. For example, place() could be return x.get(num, str(num)) -- just one line!


Your grid is a little hard to follow. It would be better to have a template and fill it in:

    template = """
-------------
| {} | {} | {} |
|-----------|
| {} | {} | {} |
|-----------|
| {} | {} | {} |
--------------"""


    if x == 2:
        # Only prints if the player has made a move
        print template.format(*(place(num + 1, pos_list) for num in range(9)))

To be frank, I agree with you comment in winner() that the code is messy. I would define tuples of valid moves and see if any of them is a match:

def winner(x,player,xx):
    wins = ((1, 2, 3), (4, 5, 6), (7, 8, 9), # Horizontal
            (1, 4, 7), (2, 5, 8), (3, 6, 9), # Vertical
            (1, 5, 9), (3, 5, 7)) # Diagonal

    if any(all(pos in x for pos in win) for win in wins):
        if xx != 1:
            print '\n'*5, "'{}'".format(player), "HAS WON!"
        return True
    return False

I changed if xx <> 1: to if xx != 1: because it's a little more obvious what it means. In fact, Python 3 took out <> so you need to use !=. I also changed your print statement. You didn't need to escape ' because it was " that made it a string, so the ' wouldn't close the string. Also, while % is not officially deprecated, it is recommended to use .format(). While I was at it, I changed return 1 == 1 to return True. Why make Python evaluate an expression when you already know the answer?


def computer_AI_part(listx):

Every function does its part. You never need the word part in its name. Also, listx isn't very descriptive. What sort of list is it? Is it a list of moves?

while True:

    # Replay's the game
    if replay == 1:
        restart = raw_input("Would you like to replay?: ")
        if restart == "yes":
            pass
        else:
            return
    else:
        print "\nTic Tac Toe - Computer vs You", '\n'*2,"Computer goes first\n"

Since replay is always either 0 or 1, and since 0 is False and 1 is True, you can simply say if replay:. I would also change the logic slightly. If I typed y or Yes, your program would just exit. I would do something more like:

while True:

    # Replay's the game
    if replay:
        restart = raw_input("Would you like to replay?: ").lower()
        if restart in ("y", "yes"): 
            pass
        elif restart in ("n", "no"):
            return
        else:
            print "Say 'yes' or 'no'"
            continue
    else:
        print "\nTic Tac Toe - Computer vs You", '\n'*2,"Computer goes first\n"

It's only barely longer, and it is much more user-friendly.


replay,computer_move,players_move,loop_count,pos_list,player_list,computer_list = 0,0,0,0,[],[],[]

That's hard to read. If you want to put them all together like that, you should at least add some whitespace and put the different types on different lines:

replay, computer_move, players_move, loop_count = 0, 0, 0, 0
pos_list, player_list, computer_list = [], [], []

That's a lot easier to read.


if computer_move in pos_list:
    continue
break

Why not:

if computer_move not in pos_list:
    break

That's a little easier to understand.


 if winner(computer_list,'Computer',2) == True:

winner() already returns a boolean. Checking its equality with True is redundant because it will result ... in the same boolean.


except:
    print "Enter a number"

You have certain expectations for what errors could happen. I would change this to:

except ValueError:
    print "Enter a number"
except (EOFError, KeyboardInterrupt):
    exit()

Yay! That fixed my original problem of not being able to exit.


if __name__ == "__main__":
    computer_and_player()

Excellent! I can import your functions without running the whole code.

\$\endgroup\$
  • \$\begingroup\$ Also when I pass computer_move as an argument and say move = x within that function, computer_move is never assigned to x. \$\endgroup\$ – TCG Apr 27 '16 at 21:34
  • \$\begingroup\$ @TCG: That's why I suggested a class. Often, you can pass a variable to a function and then reassign it to what the function returns. In this case, computer_move is used by too many things, so I would define a class and use self.computer_move instead of a global variable. \$\endgroup\$ – zondo Apr 27 '16 at 21:36

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.