2
\$\begingroup\$

I am posting this as a follow up to the Tic Tac Toe class in Python question. I have made extensive changes and rearchitected the code. I now have a board class, a game class, players and an application that ties them all together.

While at this point I have got only a 3x3 board, I have designed my code to be scalable to MxN board with K players. I am also putting up two players - random player and a second more advanced random player.

I had to repeat my code in xo.py for detecting the winner. The stateless_has_won() in meant so we may create a hypothetical move and see the result without changing the game state. Is there a better way to implement this?

board.py

#! /usr/bin/python3
from numpy import *
import math
from err import *

class board:
    def __init__(o,M,N,K,symbols):
        """
        Initilize a board of size m (rows) x n (columns), with k players playing in turn.
        Players use the symbols specified.
        """
        o.row = N
        o.col = M
        o.max_address = (M * N)
        o.num_players = K
        o.players = list(range(0,(K+1)))
        o.sym = [' ']+symbols
        o.reset()

    def reset(o):
        row = [0] * o.row
        o.board = zeros((o.col,o.row), int)
        o.free_positions = o.col * o.row

    def get_xy_from_pos(o,pos):
        x = int (math.floor( pos / o.row ))
        y = pos % o.col
        return x , y

    def set_pos(o,pos,player):
        if ( pos < o.max_address ):
            #x = int (math.floor( pos / o.row ))
            #y = pos % o.col
            x,y = o.get_xy_from_pos(pos)
            if (o.board[x][y] == 0):
                o.board[x][y] = player
                returnVal = err.OK
                o.free_positions -= 1
            else:
                returnVal = err.INVALID_MOVE
            #dbg(1,"X:" + str(x) + "  Y:" +str(y)+" R:"+str(returnVal))
        else:
            returnVal = err.INVALID_MOVE
        return returnVal

    def get_board_str(o):
        board_string = '\n'
        for j,rows in enumerate(o.board):
            for i,e in enumerate(rows):
                board_string += ' '+o.sym[e]+' '
                if i<len(rows)-1:
                    board_string += '|'
            if (j < len (o.board)-1):
                board_string += '\n---+---+---\n'
        board_string +='\n'
        return board_string

    def count_empty_squares(o):
        o.free_positions = sum(sum(o.board == 0))
        return None


def dbg(level,message):
    if (level == 1):
        print (message)


def test():
    b = board(3,3,2,['X','O'])
    b.set_pos(0,1)
    b.set_pos(0,1)

if __name__ == '__main__':
    test()

xo.py

#! /usr/bin/python3
from enum import Enum
from board import board,err
import math
from numpy import *

VERBOSE = 0

def transpose(board):
    transBoard=[[0,0,0],[0,0,0],[0,0,0]]
    c=0;
    for b in board:
        r=2
        for e in b:
            transBoard[r][c]=e;
            r -= 1
        c += 1
    return transBoard

"""
A generalized tic tac toe game class.
"""
class xo:
    def __init__(o,M,N,K,sym,order):
        """
        Initilize a game with a board of size M x N with K players using the symbols in the list sym.
        The players play in the order specified by the list order.
        """
        o.brd = board(3, 3, 2, sym)
        o.play_order = order
        o.num_players = K
        o.reset()

    def reset(o):
        o.brd.reset()
        o.game_turn = 0
        o.gameLog = ones((9,2),int)*-1
        o.game_over = False
        o.turn = o.play_order[0]

    def mark(o,pos,player):
        """
        Marks a position on the board with the symbol for the player.

        If the position is already marked, the function returns err.INVALID_MOVE

        If a player attempts to play out of turn, the function returns err.OUT_OF_TURN
        """
        # Are we trying to play after the game is over?
        if (o.game_over == False):
            # Is a player trying to play out of turn?
            if (player == o.play_order[(o.game_turn % o.num_players)]):
                returnVal = o.brd.set_pos(pos,player)
                # is the postion selected to mark is not invalid?
                if (returnVal == err.OK):
                    o.gameLog[o.game_turn] = [player, pos]
                    o.game_turn += 1
                    # Do we have a winner?
                    returnVal = o.has_won(player)
                    if (returnVal == err.WIN):
                        # Yes. We do have a winner
                        o.game_over = True
            else:
                # Cheat! Did you think you could get away by playing out of turn? Well, you cant!
                console_log(1,"OUT OF TURN")
                returnVal = err.OUT_OF_TURN
        else:
            # yes. A player tried to play after the game was over... Droids !!!
            returnVal = err.INVALID_MOVE
        return returnVal

    def get_board(o):
        return o.brd.get_board_str();

    def has_won(o,player):
        """
        Implemented for standard 3x3 tic tac toe game
        """
        if (o.brd.free_positions == 0):
            o.game_over = True
        win_logic = (o.brd.board == player)
        # Check linear
        for i in range(0,2):
            lin_sum = sum ( sum(win_logic,i) == 3 )
            if (lin_sum == 1):
                returnVal=err.WIN;
                break
            else:
                returnVal = err.OK
        # check diagonals
        if (returnVal == err.OK):
            if (((sum(diagonal(win_logic)))==3) | ((sum(diagonal(transpose(win_logic))))==3) ):
                returnVal=err.WIN
        if ((o.game_over == True) & (returnVal == err.OK)):
            returnVal = err.DRAW
        return returnVal;

    def stateless_has_won(o,board,player):
        """
        Implemented for standard 3x3 tic tac toe game
        """
        game_over = False
        free_positions = sum(sum(board == 0))
        if (free_positions == 0):
            game_over = True
        win_logic = (board == player)
        # Check linear
        for i in range(0,2):
            lin_sum = sum ( sum(win_logic,i) == 3 )
            if (lin_sum == 1):
                returnVal=err.WIN;
                break
            else:
                returnVal = err.OK
        # check diagonals
        if (returnVal == err.OK):
            if (((sum(diagonal(win_logic)))==3) | ((sum(diagonal(transpose(win_logic))))==3) ):
                returnVal=err.WIN
        if ((game_over == True) & (returnVal == err.OK)):
            returnVal = err.DRAW
        return returnVal;

def console_log(level,log_line):
    global VERBOSE
    if level <= VERBOSE:
        print (log_line)

def main():
    print("\nTic Tac Toe Platform Test\n_________________________")
    g=xo(3,3,2,['X','O'],[2,1])
    print(g.get_board())

    g.mark(0,1)
    print(g.get_board())
    g.mark(0,2)
    print(g.get_board())
    g.mark(2,2)
    print(g.get_board())
    g.mark(3,1)
    print(g.get_board())
    g.mark(4,2)
    print(g.get_board())

    print ("Game Log:"+str(g.gameLog))

if __name__ == '__main__':
    if __doc__:
        print ('Module:')
        print (__doc__ + '\n')
    main()

err.py

#! /usr/bin/python3
from enum import Enum

class err(Enum):
    OK = 0
    WIN = 1
    DRAW = 2
    OUT_OF_TURN = -1
    INVALID_MOVE = -2

random1SPlayer

#! /usr/bin/python3

from random import randint
from err import err

from copy import *
from numpy import *

class random1SPlayer:
    """
    The random player with 1 step look ahead finds a list of valid moves, and 
    chooses one among it such that it can win the next move, or prevent an 
    adversary from winning in the next move.

    Essentially this is a 1 level min max algorithm.

    """
    def __init__(o,play_as,name,game):
        o.play_as = play_as;
        o.name = name
        o.title = "Random Player with one step look ahead"
        o.game = game

    def play(o):
        # Compute possible moves
        possible_moves=[];
        n = 0
        board = deepcopy(o.game.brd.board)
        for b in board:
            for e in b:
                if (e == 0):
                    possible_moves=possible_moves+[n]
                n += 1
        move = -1
        # For each of the listed move, see if the player can win in the next move
        for m in possible_moves:
            new_board = deepcopy(board)
            x,y=o.game.brd.get_xy_from_pos(m)
            new_board[x][y]=o.play_as;
            result = o.game.stateless_has_won(new_board,o.play_as)
            if (result == err.WIN):
                move = m;
                break
        # We cannot win in the next move.
        if (move == -1):
            # See if our adversary can win if give a chance to play.
            # Compute the adversary's id
            temp = roll(o.game.play_order,1)
            adversary=temp[o.play_as-1]
            for m in possible_moves:
                new_board = deepcopy(board)
                x,y=o.game.brd.get_xy_from_pos(m)
                new_board[x][y]=adversary;
                result = o.game.stateless_has_won(new_board,adversary)
                if (result == err.WIN):
                    move = m;
                    break
        if (move == -1):
            move = possible_moves[randint(0, len(possible_moves)-1)]
        else:
            move = m
        return int(move)

def main():
    pass

if __name__ == '__main__':
    main()    

randomPlayer.py

#! /usr/bin/python3

from random import randint

class randomPlayer:
    """
    The random player finds a list of valid moves, and chooses one among it.
    """
    def __init__(o,play_as,name,game):
        o.play_as = play_as;
        o.name = name
        o.title = "Random Player"
        o.game = game

    def play(o):
        possible_moves=[];
        n = 0
        board = o.game.brd.board
        for b in board:
            for e in b:
                if (e == 0):
                    possible_moves=possible_moves+[n]
                n += 1
        move = possible_moves[randint(0, len(possible_moves)-1)]
        return int(move)

def main():
    player=randomPlayer(1);
    print( player.play([[0,0,0],[0,0,0],[0,0,0]]) )
    print( player.play([[0,1,0],[0,0,0],[0,0,0]]) )
    print( player.play([[0,1,2],[0,0,0],[0,0,0]]) )
    print( player.play([[0,1,2],[1,2,0],[0,0,0]]) )

if __name__ == '__main__':
    main()    

The Application

#! /usr/bin/python3
from xo import xo
from humanPlayer import humanPlayer
from randomPlayer import randomPlayer
from random1SPlayer import random1SPlayer
from err import err

from random import randint
from numpy import *

VERBOSE = 2
N_GAMES = 1000
ROLL = False
LINE = "-----------------------------------------"

def main():
    global LINE
    PLAY_ORDER = [1,2]
    g=xo(3,3,2,['X','O'],PLAY_ORDER)
    player1 = random1SPlayer(1,"Rand 1",g)
    player2 = random1SPlayer(2,"Rand 2",g)  
    print('\nTic Tac Toe Platform\n'+player1.title+' vs '+player2.title)
    console_log(0,LINE)

    rand1_win = 0
    rand1_win_turns = 0
    rand2_win = 0
    rand2_win_turns = 0
    draw = 0

    f = open('game_log_rand_rand1.csv', 'w')
    # Play N Games
    for i in range (0, N_GAMES):
        csv_line = ''
        g.reset()
        current_winner = 0
        # create a game instance
        if ROLL :
            PLAY_ORDER = roll (PLAY_ORDER,1)
        # Keep playing till the game in not over.
        while(g.game_over == False):
            move = player1.play()
            result = g.mark(move,1)
            console_log (3,'Result: '+str(result)+"\n")
            if (result == err.DRAW):
                console_log (2, g.get_board())
                current_winner = 0;
                draw += 1
            if (result == err.WIN):
                console_log (2, '______________________')
                console_log (2, g.get_board())
                console_log (1, player1.name+' has won')
                current_winner = 1
                console_log (2, '______________________')
                rand1_win += 1
                rand1_win_turns += g.game_turn
            if (g.game_over == False):
                move = player2.play()
                result = g.mark(move,2)
                console_log (3,'Result: '+str(result)+"\n")
                if (result == err.DRAW):
                    console_log (2,g.get_board())
                    current_winner = 0
                    draw += 1
                if (result == err.WIN):
                    console_log (2, "______________________")
                    console_log (2, g.get_board())
                    console_log (1, player2.name+" has won")
                    console_log (2, "______________________")
                    current_winner = 2
                    rand2_win += 1
                    rand2_win_turns += g.game_turn;
        # Log game to CSV file
        for i, e in enumerate(g.play_order):
            csv_line += str(e)+','
        csv_line += str(current_winner)
        for e in g.gameLog[:,1]:
            csv_line += ', '+str(e)
        csv_line += '\n'
        f.write(csv_line)
    f.close()
    console_log(1,LINE)
    print (player1.name+" Winner :"+str(rand1_win))
    if (rand1_win>0):
        print ("Average Moves to win :"+str(rand1_win_turns/rand1_win))
    print (player2.name+" Winner :"+str(rand2_win))
    if (rand2_win>0):
        print ("Average Moves to win :"+str(rand2_win_turns/rand2_win))
    print ("Draw :"+str(draw))

def console_log(level,log_line):
    global VERBOSE
    if level <= VERBOSE:
        print (log_line)

if __name__ == '__main__':
    main()
\$\endgroup\$
  • \$\begingroup\$ You are still not following PEP-8 (e.g. on whitespace) and not using self for the instance is strongly discouraged... \$\endgroup\$ – jonrsharpe Sep 17 '15 at 20:49
  • \$\begingroup\$ Oh.. That was a MATLAB habit. I'll do a find and replace. Let me read though pep8 and see about the space. \$\endgroup\$ – Lord Loh. Sep 17 '15 at 22:41
  • \$\begingroup\$ Just to document - pypi.python.org/pypi/pep8 is useful to check the script :-) \$\endgroup\$ – Lord Loh. Sep 20 '15 at 6:56
2
\$\begingroup\$

Wow, that’s a lot of code! Rather than read it all, I’m just going to try running the final file:

$ python tictactoe.py
Traceback (most recent call last):
  File "tictactoe.py", line 3, in <module>
    from humanPlayer import humanPlayer
ImportError: No module named 'humanPlayer'

*is sad*

But upon inspection, we aren’t actually using that class, so I’ll just remove the import line.

Wow, lots of input just flew past very quickly! Let’s inspect a few of the boards which just went across my screen:

  • A game which ended in victory:

     O | X |
    ---+---+---
     X | O | X
    ---+---+---
     X | O | O
    
    Rand 2 has won
    

    The board is drawn in terms of O’s and X’s, but the players are numbered. Perhaps it would be better to name the players after their symbols?

  • A game which ended in a draw:

     X | O | X
    ---+---+---
     X | X | O
    ---+---+---
     O | X | O
    

    There’s no result printed after this. It would be nice if there was a short sentence after tied results, like there is when the game ends in a victory, to save me having to check the board.

  • I cannot divine what causes a horizontal line to be printed or not printed between two games. Some games are separated by two lines, some by one, some by none. It should be consistent.

At over five thousand lines of output, it’s scrolled off the top of my screen and into the abyss; I can only review the last few hundred games. I’m not convinced this output is particularly useful (I’m not going to inspect every case); I think it might be better funnelled into a separate file. It can be looked at if necessary, but ignored in most cases.

Finally, I get this summary-like text:

Rand 1 Winner :319
Average Moves to win :7.282131661442007
Rand 2 Winner :183
Average Moves to win :8.0
Draw :498

I don’t have any context for these numbers. I can work it out by looking at the rest of the output and glancing at the code, but it would be better if this output stood on its own. I also think it could be tidied up slightly.

More like:

The computer played 1000 games of random tic-tac-toe. These are the results:

Player 1:    319 wins       (avg. 7.28 moves to win)
Player 2:    183 wins       (avg. 8.0  moves to win)
Draws:       498


That’s probably enough trawling through the output. I don’t have time to do a thorough review of all 325 lines of code, so here are some suggestions based on a quick skim:

  • There isn’t much in the way of comments or documentation. That would make the code much easier to follow. One simple example: the constructor for the xo class:

    def __init__(o,M,N,K,sym,order):
    

    What do any of those arguments mean? What should I do with them? I have to dive into the code to find out.

  • Read PEP 8, the Python style guide. Among other things that need fixing:

    • The first argument in class methods should be self, not o.
    • Python naming convention is snake_case for variables, UPPERCASE for constants and CamelCase for class names.
    • Spaces after commas and around operators.

  • I’m not a fan of using your err enum to give meaning to return codes, and using that to dictate control flow. Better to define some custom exceptions, and use try … catch accordingly.

  • When comparing to True/False, rather than using

    if (o.game_over == False)
    

    you can just do:

    if not self.game_over:
    

    That’s cleaner and more Pythonic.

  • Rather than defining your own console_log function, it would be better to use something like the logging module. Python comes with “batteries included” – so use them!

  • In the main file, you have this block of code:

    f = open('game_log_rand_rand1.csv', 'w')
    # lots of code
    f.close()
    

    The more Pythonic approach is to use a context manager:

    with open('game_log_rand_rand1.csv', 'w') as f:
        # lots of code
    

    That ensures the file will be closed correctly, even if an exception gets raised midway through.

    Your program is doing a lot of work while the file is open, most of which doesn’t need the file. I prefer to minimise the amount of work done while files are open, so that I can quickly find and inspect any code that’s actually interacting with files.

  • There’s a lot of repeated code for printing a board and a result. It would be better to have a generic function that takes a game, and logs/prints the relevant information in a consistent manner.

  • Try to use descriptive variable names rather than single letters. If I can easily work out what your variables correspond to, then your code is significantly easier to read, review and debug.

\$\endgroup\$
  • \$\begingroup\$ Wow !! Thank you for the detailed review and comments. I should have made your life easier by providing the repository - github.com/lordloh/xo I shall make the changes and probably post a follow up. The readme in the repository would have been helpful to explain my work. I also wanted to know if I have architected the game class, the board class, the player class and the application well ?? can there be a better way to interface them? \$\endgroup\$ – Lord Loh. Sep 19 '15 at 4:41

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.