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

Tant Fant Game in Python

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

Problem

Here is our code of Tant Fant Game. Does anyone have suggestions for how to improve our code?

```
class board:
loc = []
def __init__(self) :
self.loc = ['B', 'B', 'B', '', '', '', 'W', 'W', 'W']

def cur_state(self) :
return self.loc

def find_empty_spot(self) :
return [i + 1 for i, j in enumerate(self.cur_state()) 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) :
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())

if move == 'right' :
next_pos = pos + 1
if next_pos in empty_spot :
update_board.set_piece(pos, '')
update_board.set_piece(next_pos, cur_piece)
return update_board
elif move == 'left' :
next_pos = pos - 1
if next_pos in empty_spot :
update_board.set_piece(pos, '')
update_board.set_piece(next_pos, cur_piece)
return update_board
elif move == 'up' :
next_pos = pos - 3
if next_pos in empty_spot :
update_board.set_piece(pos, '')
update_board.set_piece(next_pos, cur_piece)
return update_board
elif move == 'down' :
next_pos = pos + 3
if next_pos in empty_spot :
update_board.set_piece(pos, '')
update_board.set_piece(next_pos, cur_piece)
return update_board
elif move == 'upleft' :
next_pos = pos - 4
if next_pos in empty_spot :
update_board.set_piece(pos, '')
update_board.set_piece(next_pos, cur_piece)
ret

Solution

Fail Fast, Fail Loudly

If move is not an allowed value the function just return None, instead you could make it fail loudly with a clear error message:

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")
    # Rest of function


Avoidance of repetition

elif move == 'down' :
        next_pos = pos + 3
        if next_pos in empty_spot :
            update_board.set_piece(pos, '')
            update_board.set_piece(next_pos, cur_piece)
            return update_board
    elif move == 'upleft' :
        next_pos = pos - 4
        if next_pos in empty_spot :
            update_board.set_piece(pos, '')
            update_board.set_piece(next_pos, cur_piece)
            return update_board
    ...


This block is repeated every time:

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


So you can write it to the bottom:

if move == 'right' :
        next_pos = pos + 1
    elif move == 'left' :
        next_pos = pos - 1
    elif move == 'up' :
        next_pos = pos - 3
    # All other if branches containing only `next_pos` assignement

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

Code Snippets

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")
    # Rest of function
elif move == 'down' :
        next_pos = pos + 3
        if next_pos in empty_spot :
            update_board.set_piece(pos, '')
            update_board.set_piece(next_pos, cur_piece)
            return update_board
    elif move == 'upleft' :
        next_pos = pos - 4
        if next_pos in empty_spot :
            update_board.set_piece(pos, '')
            update_board.set_piece(next_pos, cur_piece)
            return update_board
    ...
if next_pos in empty_spot :
            update_board.set_piece(pos, '')
            update_board.set_piece(next_pos, cur_piece)
            return update_board
if move == 'right' :
        next_pos = pos + 1
    elif move == 'left' :
        next_pos = pos - 1
    elif move == 'up' :
        next_pos = pos - 3
    # All other if branches containing only `next_pos` assignement

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

Context

StackExchange Code Review Q#147013, answer score: 6

Revisions (0)

No revisions yet.