HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonModerate

TicTacToe where the computer plays random moves against itself

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
randomitselfthemovestictactoewhereplaysagainstcomputer

Problem

I wrote a quick program in Python to play against itself in a game of (random) tic-tac-toe.

It seems to work, but can you guys please look at it and let me know if I'm doing things the 'correct' (i.e. idiomatic) and efficient way.

import random

#  Tic Tac Toe

#  Board is laid out as:
#  0, 1, 2,
#  3, 4, 5,
#  6, 7, 8

winning_rows = [
    [0, 1, 2], [3, 4, 5], [6, 7, 8],    # Horizontal
    [0, 3, 6], [1, 4, 7], [2, 5, 8],    # Vertical
    [0, 4, 8], [2, 4, 6]                # Diagonal
    ]

def check_winner(board):
    for player in [1, 2]:
        for seq in winning_rows:
            for pos in seq:
                if (board[pos]!=player):
                    break
            else:
                return player

    #nobody won, check for draw (return 0 for draw, None for incomplete game)
    for pos in range(9):
        if (board[pos] == 0):
            return None

    #game is a draw
    return 0

def find_move(board, player):
    legal_moves = []

    for pos in range(9):
        if (board[pos] == 0):
            legal_moves.append(pos)

    return random.choice(legal_moves)

def make_move(board, move, player):
    board[move] = player

def write_board(board):
    #letter = {0: '-', 1: 'X', 2: 'O'}
    letter = "-XO"
    for row in range(3):
        for col in range(3):
            print letter[board[row*3+col]], 
        print
    print

def play():
    board = []
    for pos in range(9):
        board.append(0)

    write_board(board)  

    player = 1
    while True:
        move = find_move(board, player)
        make_move(board, move, player)
        write_board(board)
        winner = check_winner(board)
        if winner != None:
            if winner == 1:
                print "Player 1 (X) wins!"
            if winner == 2:
                print "Player 2 (O) wins!"
            if winner == 0:
                print "Game is a draw!"
            break
        player = 3 - player

play()

Solution

-
Some of your functions can use list comprehensions.

As you seem to know maths, you may have come across 'set-builder notation',
and that's what list comprehension are based off:

\$S = \{2 \times x | x \in \{0, 1, ..., 9\}, x^2 > 3\}\$

Which in Python is:

[2 * x for x in range(10) if x ** 2 > 3]


I would use it in:

-
find_move for an improvement in clarity.

def find_move(board, player):
    return random.choice([pos for pos in range(9) if board[pos] == 0])


-
write_board to use less prints. It's harder to read however.

print '\n'.join([
    ''.join([letter[row + col] for col in range(3)])
    for row in range(0, 3, 9)
])


-
check_winner you can make your for pos in seq one.
Using all.

for player in [1, 2]:
    for seq in winning_rows:
        if all([board[pos] == player for pos in seq]):
            return player


-
I'm surprised you didn't make a new_board function. Which could be:

def new_board():
    return [0 for _ in range(9)]


Alternatively, as noted by @i-live-in-a-storm-drain, if you are ok with shallow copy's you could change it to:

def new_board():
    return [0] * 9


The down-side to this is if you mutate any of the items, then you mutate all of them. For example you wouldn't want to make a 2D list with it.

>>> a = [[0] * 3] * 3
>>> a[0][0] = 1
>>> a
[[1, 0, 0], [1, 0, 0], [1, 0, 0]]


-
Your function make_move seems, kinda pointless...
I would use board[move] = player over the function.

-
You should use is when comparing to singletons, None.

# Bad
if winner != None:

# Good
if winner is not None:


-
You could make the win conditions a list, and print from that.

win_statments = ["Player 1 (X) wins!", "Player 2 (O) wins!", "Game is a draw!"]
print win_statments[winner]


-
For check_winner you can use the fact 0 == False (Not 0 is False).
With Python's builtin all to check if all of them are not 0.

if not all(board):
    return None


-
As this is Python2 it's recommended to use xrange rather than range for almost everything.
You'll get a, minimal, speed increase in most situations.

However as noted by @Janos, you could opt to keep them as range, for an easier transition to Python3. This is as there is no xrange in Python3.

You should only need to change your prints to functions for this to work in Python3.

# Change to these:
print("Player 1 (X) wins!")

# in `make_move`
for col in range(3):
    print(letter[board[row*3+col]], end='')
print()

Code Snippets

[2 * x for x in range(10) if x ** 2 > 3]
def find_move(board, player):
    return random.choice([pos for pos in range(9) if board[pos] == 0])
print '\n'.join([
    ''.join([letter[row + col] for col in range(3)])
    for row in range(0, 3, 9)
])
for player in [1, 2]:
    for seq in winning_rows:
        if all([board[pos] == player for pos in seq]):
            return player
def new_board():
    return [0 for _ in range(9)]

Context

StackExchange Code Review Q#113840, answer score: 11

Revisions (0)

No revisions yet.