4
\$\begingroup\$

Last time I posted my code of Tant Fant Game. I got some really good advice and I have implemented all the suggested changes to my code. I was wondering if there were any other revisions that would be good to implement? Thanks! =)

class board:

    def __init__(self) :
        self.loc = ['B', 'B', 'B', '', '', '', 'W', 'W', 'W']
        self.translation = {'right' : 1, 
                            'left' : -1, 
                            'up' : -3, 
                            'down' : 3, 
                            'upleft' : -4, 
                            'upright' : -2, 
                            'downleft' : 2, 
                            'downright' : 4}        
    def cur_state(self) :
        return self.loc

    def find_empty_spot(self) :
        return [i for i, j in enumerate(self.cur_state(), 1) if j == '']

    def set_piece(self, pos, piece) :
        self.loc[pos-1] = piece

    def set_state(self, state) :
        for i in range(len(state)) :
            self.loc[i] = state[i]

    # update the board
    def do_move(self, pos, move, cur_board) :
        if move not in ['up', 'down', 'left', 'right', 'upleft', 'upright', 'downleft', 'downright']:
            raise ValueError("Move not in allowed options")
        empty_spot = cur_board.find_empty_spot()
        cur_piece = cur_board.loc[pos-1]
        update_board = board()
        update_board.set_state(cur_board.cur_state())

        try:
            next_pos = pos + self.translation[move]
        except KeyError:
            raise ValueError("Move not in allowed options")

        if next_pos in empty_spot :
            update_board.set_piece(pos, '')
            update_board.set_piece(next_pos, cur_piece)
            return update_board

    # black piece : 'X'
    # white piece : 'W'
    def print_board(self) :
        next_line = 1
        cur_loc = self.cur_state()
        # print cur_loc
        for pos in cur_loc :
            if pos == 'B' :
                print 'X |',
            elif pos == 'W' :
                print 'O |',
            else :
                print '  |',
            if next_line % 3 == 0 :
                print ""
                print "-----------"
                next_line = 1
            else :
                next_line += 1

class player:

    black_player_pieces = 3
    white_player_pieces = 3
    cur_turn = ''
    move_rules = {1 : ['right', 'down', 'downright',], 2 : ['left', 'right', 'down'], 3 : ['left', 'down', 'downleft'], 4 : ['up', 'down', 'right'], 5 : ['up', 'down', 'left', 'right', 'upleft', 'upright', 'downleft', 'downright'], 6 : ['left', 'up', 'down'], 7 : ['up', 'right', 'upright'], 8 : ['up', 'left', 'right'], 9 : ['up', 'upleft', 'left']}

    def __init__(self) :
        self.cur_turn = 'W'

    def next_turn(self, turn) :
        if turn == 'W' :
            self.cur_turn = 'B'
        else :
            self.cur_turn = 'W'

    def gen_move(self, pos, board) :
        turn = self.cur_turn
        self.next_turn(turn)
        next_moves = self.move_rules[pos]
        return [next_moves, pos, board]

    def primitive(self, cur_board) :
        def all_same_pieces(line) :
            elem = line[0]
            return all(x == elem for x in line)

        def who_wins(self) :
            if self.cur_turn == 'W' :
                return 'B'
            else :
                return 'W'

        state = cur_board.cur_state()
        checklists = [[state[0], state[4], state[8]], [state[3], state[4], state[5]], [state[2], state[4], state[6]], [state[0], state[3], state[6]], [state[1], state[4], state[7]], [state[2], state[5], state[8]]]
        res = "UNDECIDED"
        for each_list in checklists :
            if "" in each_list :
                continue
            if all_same_pieces(each_list) == False :
                continue 
            res = who_wins(self)
        return res


def main() :
    print "start Tant Fant"
    tf_board = board()
    tf_player = player()
    tf_board.print_board()
    while True :
        res = tf_player.primitive(tf_board)
        if res != 'UNDECIDED' :
            print res,
            print "WIN!"
            return
        print "which side to play?",
        print tf_player.cur_turn
        pos = input("input your position:")
        next_moves, pos, tf_board = tf_player.gen_move(pos, tf_board)
        print "next_moves: ",
        print next_moves
        move = raw_input("input your move: ")
        tf_board = tf_board.do_move(pos, move, tf_board)
        tf_board.print_board()


    '''
    tant_fant_board = board()
    tant_fant_player = player()
    next_moves, cur_pos, cur_board = tant_fant_player.gen_move(1, tant_fant_board)
    for move in next_moves :
        update_board =  tant_fant_board.do_move(cur_pos, move, cur_board)
        if update_board :
            update_board.print_board()
    '''

    print "game over"

main()
\$\endgroup\$
3
\$\begingroup\$

2 dimensional board

Tant Fant is played on a 2-dimensional board, representing it one 1 dimension leads to unnecessary complication, like this list of allowed moves:

move_rules = {1 : ['right', 'down', 'downright',], 2 : ['left', 'right', 'down'], 3 : ['left', 'down', 'downleft'], 4 : ['up', 'down', 'right'], 5 : ['up', 'down', 'left', 'right', 'upleft', 'upright', 'downleft', 'downright'], 6 : ['left', 'up', 'down'], 7 : ['up', 'right', 'upright'], 8 : ['up', 'left', 'right'], 9 : ['up', 'upleft', 'left']}

Instead you in 2-d you could just check if the result index is inside the board.

All Mutable XOR All Immutable

The board class both has methods to change its internal state:

def set_piece(self, pos, piece) :
    self.loc[pos-1] = piece

def set_state(self, state) :
    for i in range(len(state)) :
        self.loc[i] = state[i]

And a method that return a whole new object:

def do_move(self, pos, move, cur_board) :
    if move not in ['up', 'down', 'left', 'right', 'upleft', 'upright', 'downleft', 'downright']:
        raise ValueError("Move not in allowed options")
    empty_spot = cur_board.find_empty_spot()
    cur_piece = cur_board.loc[pos-1]
    update_board = board()
    update_board.set_state(cur_board.cur_state())

    try:
        next_pos = pos + self.translation[move]
    except KeyError:
        raise ValueError("Move not in allowed options")

    if next_pos in empty_spot :
        update_board.set_piece(pos, '')
        update_board.set_piece(next_pos, cur_piece)
        return update_board

You should be consistent in implementing mutability or immutability.

__str__

An object in Python knows how it should be converted to string, you just need to supply the correct __str__ method.

This is considered more re-usable and idiomatic than a print_object method.

Also your print_board method can be modularized:

def lines(self):
    b = self.loc
    return [ [b[0],b[1],b[2]], [b[3],b[4],b[5]], [b[6],b[7],b[8]]]

@staticmethod
def str_line(line):
    """
    >>> board.str_line(["B", "W", ""])
    'X | O |   | '
    """
    return ''.join('X | ' if x == 'B' else 'O | ' if x == 'W' else '  | ' for x in line)


def __str__(self) :
    """
    >>> print(board())
    X | X | X | 
    -----------
      |   |   | 
    -----------
    O | O | O | 
    -----------

    """
    return '\n'.join(board.str_line(line) + "\n-----------" for line in self.lines())

This method is both faster to read and to develop, when you write a more smaller functions you can test smaller parts of behaviour, and continue building your program confident that the previous parts work as intended.

Reading also becomes easier because each function builds on the previous one instead of starting from scratch.

primitive ?

def primitive(self, cur_board) :
    def all_same_pieces(line) :
        elem = line[0]
        return all(x == elem for x in line)

    def who_wins(self) :
        if self.cur_turn == 'W' :
            return 'B'
        else :
            return 'W'

    state = cur_board.cur_state()
    checklists = [[state[0], state[4], state[8]], [state[3], state[4], state[5]], [state[2], state[4], state[6]], [state[0], state[3], state[6]], [state[1], state[4], state[7]], [state[2], state[5], state[8]]]
    res = "UNDECIDED"
    for each_list in checklists :
        if "" in each_list :
            continue
        if all_same_pieces(each_list) == False :
            continue 
        res = who_wins(self)
    return res

First of all what does primitive stand for? Why is checking who wins a primitive behaviour? Please change to a better name.

I also have many comments about this function:

    def who_wins(self) :
        if self.cur_turn == 'W' :
            return 'B'
        else :
            return 'W'

who_wins(self) is the same as next_turn(self, turn) (turn argument is never used in the latter). So you should write:

who_wins = self.next_turn

To avoid duplication (or just not define it at all). (And I suggest using a ternary for conciseness).

Now onto this list:

checklists = [[state[0], state[4], state[8]], [state[3], state[4], state[5]], [state[2], state[4], state[6]], [state[0], state[3], state[6]], [state[1], state[4], state[7]], [state[2], state[5], state[8]]]

You can only hope it is correct, and it also without explanation.

I would do it like this (in the board class)

def lines(self): # Already defined, see above
def columns(self):
    return zip(* self.lines())
def diagonals(self):
    b = self.loc
    return [[b[0], b[5], b[8]], [b[2], b[4], b[6]]]

Each one of this can be tested individually, and now the checklist actually makes sense:

checklist = board.lines() + board.columns() + board.diagonals()

Also the None convention:

res = "UNDECIDED"

None is used to signify undecided/not given so you should use it instead of an arbitrary string.

And finally you can simplify logic flow in the last loop:

for triplet in checklist:
    if all_same_pieces(triplet) == False :
        return who_wins(self) # self.next_turn()

Naming

Class names start with a capital letter Board, Player. This is a widely accepted convention you should follow. (see PEP8)

\$\endgroup\$

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.