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()
asked Sep 17, 2015 at 18:32
\$\endgroup\$
3
  • \$\begingroup\$ You are still not following PEP-8 (e.g. on whitespace) and not using self for the instance is strongly discouraged... \$\endgroup\$ Commented Sep 17, 2015 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\$ Commented Sep 17, 2015 at 22:41
  • \$\begingroup\$ Just to document - pypi.python.org/pypi/pep8 is useful to check the script :-) \$\endgroup\$ Commented Sep 20, 2015 at 6:56

1 Answer 1

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.

answered Sep 18, 2015 at 22:16
\$\endgroup\$
1
  • \$\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\$ Commented Sep 19, 2015 at 4:41

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.