I am self-teaching Python. I wrote a simple and extendable version of Tic-Tac-Toe. As far as I know, I did some immature handling with too many function parameters and multiple creation of list.
Please let me know your thoughts on this implementation and how to make it better?
def create_board(board_size,to_index=False):
if to_index:
return [list(x for x in range(y*board_size,(y+1)*board_size)) for y in range(0,board_size)]
return [list(x+1 for x in range(y*board_size,(y+1)*board_size)) for y in range(0,board_size)]
def print_board(board):
print('current board is:')
for row in board:
to_print='|'+''.join([str(s)+'|' for s in row])
print(to_print)
def create_and_print_board(board_size):
print('Creating %dx%d board' %(board_size,board_size))
board = create_board(board_size)
print_board(board)
return board
def player_entry(num):
user = "User{d}".format(d=num)
ask_input = "{u}, enter your symbol ".format(u=user)
symbol= input(ask_input)
print('%s, your symbol is %s ' %(user,symbol))
return symbol
def board_to_flat(board):
new_list= []
for x in board:
new_list += x
return new_list
def get_valid_indice(player, curr_indices):
to_ask='Player{p}, please choose from available indices to play '.format(p=player)
print(curr_indices)
player_input = input(to_ask)
if player_input.isnumeric():
i_input=int(player_input)
if curr_indices.count(i_input)!= 0:
print('Player%d, you have chosen index %d ' %(player,i_input-1))
return i_input-1
print('Player%d, you have chosen invalid index %s ' %(player,player_input))
print('please enter again!!')
return get_valid_indice(player,curr_indices)
def get_indices(board):
board_ind = board_to_flat(create_board(len(board)))
to_indices=[x for x in board_to_flat(board) if board_ind.count(x)==1 ]
return to_indices
def ask_player_for_index(player,board):
print_board(board)
curr_indices=get_indices(board)
if len(curr_indices)==0:
return -1
valid_indice = get_valid_indice(player,curr_indices)
return valid_indice
def add_index_to_board(board,player_index,player_symb):
p_in=player_index+1
board_len = len(board)
te_in=int(p_in/board_len)
rel_in=te_in*board_len
col_in=player_index-rel_in
if col_in < 0:
te_in =te_in-1
rel_in=te_in*board_len
col_in=player_index-rel_in
board[te_in][col_in]=player_symb
print('%s symbol was added to board at [%d][%d] ' %(player_symb,te_in,col_in))
def merge(main_list,multi_list,b_len):
for x in [s for s in multi_list if len(s)==b_len]:
main_list.append(x)
def passing_comb(board):
max_col = len(board)
max_row = len(board[0])
cols = [[] for i in range(max_col)]
rows = [[] for i in range(max_row)]
fdiag = [[] for i in range(max_col + max_row - 1)]
bdiag = [[] for i in range(len(fdiag))]
min_bdiag = -max_col + 1
for y in range(max_col):
for x in range(max_row):
cols[y].append(board[y][x])
rows[x].append(board[y][x])
fdiag[x+y].append(board[y][x])
bdiag[-min_bdiag+x-y].append(board[y][x])
res=[]
merge(res,cols,max_col)
merge(res,rows,max_col)
merge(res,fdiag,max_col)
merge(res,bdiag,max_col)
return res
def has_player_won(board,player_symb):
board_len=len(board)
pass_res=player_symb*len(board)
build_board = create_board(board_len,True)
comb = passing_comb(build_board)
flat_board = board_to_flat(board)
#print([s for s in comb])
has_won = False
for k in comb:
curr_res = ''.join([str(flat_board[p]) for p in k])
#print(curr_res)
if curr_res==pass_res:
return True
return False
def player_play_game(player,board,player_symb):
player_index= ask_player_for_index(player,board)
if player_index !=-1:
add_index_to_board(board,player_index,player_symb)
has_won= has_player_won(board,player_symb)
if has_won:
print('Player%d, you Win' %(player))
print_board(board)
return has_won or player_index==-1
def play_game(player_one,player_two,player_one_symb,player_two_symb,board):
end_of_game = False
while(not end_of_game):
end_of_game=player_play_game(player_one,board,player_one_symb)
if end_of_game:
break
end_of_game=player_play_game(player_two,board,player_two_symb)
else:
print('Game is finished')
if not has_player_won(board,player_one_symb) and not has_player_won(board,player_two_symb):
print('Its Draw')
def main():
board_size=4
player_one=1
player_two=2
print('Welcome to TicTacToe Game!! ')
print('------------------------------')
board = create_and_print_board(board_size)
print('------------------------------')
player_one_symb =player_entry(player_one)
print('------------------------------')
player_two_symb =player_entry(player_two)
print('------------------------------')
play_game(player_one,player_two,player_one_symb,player_two_symb,board)
print('------------------------------')
main()
-
\$\begingroup\$ I mean it can be scaled to NXN board. Sorry for poor vocabulary. \$\endgroup\$plotop– plotop2018年05月03日 07:29:23 +00:00Commented May 3, 2018 at 7:29
1 Answer 1
Before I would make any suggestion with respect to specific lines of code, I would first recommend an entirely different approach. It is really hard to read your code and get the 'big picture' of what is going on. This is a perfect example of using OO design to add context to your code as well as separations of concerns. I've thrown together just a rough example of how you might think about it differently.
If you were to describe your program to someone, a few nouns would jump out. You have a Board, you have a Player, you have something that moderates the game, and you even have pieces (though I didn't model them). Think about the 'things' in your programs as classes.
In each class, place code that pertains to the thing. In the Board class, place 'boardy' things. In the player class, place 'player' things, and so on. What are boardy and player things is something that experience will teach you, but thinking this way will get you closer.
If you look at my code, it becomes apparent almost immediately what is going on. My system becomes almost obvious without having to explain it. Granted, mine is missing a lot of stuff, but there should be enough there to paint a picture for you.
If someone were to look at my code later, and need to know why large boards are not being created correctly - she/he would quickly find the Board class, and almost as quickly find the create_board method. Wouldn't you agree that your way would require a lot more searching?
class Board:
def __init__(self, size):
# create the board here
self.board = self._create_board(size)
def __str__(self):
# print the board here
pass
@property
def available_spaces(self):
# return a list of available places (indices) here
return []
@property
def game_over(self):
return False # Change this to logic to see if game over
def is_space_taken(self, index):
return True # Chaneg this to logic to see if space available
def place_piece(self, index: int, symbol: str):
# user fills a piece of the board
# throw an exception if index not available
pass
def _create_board(self, size: int):
# create your board here
return [[]]
class Player:
def __init__(self, name: str, symbol: str):
self.symbol = symbol
class Moderator:
def __init__(self):
pass
def new_game(self):
# get board size and create board
board = Board(3) # whatever size the user chooses
# create player 1
player1 = Player("Player1", "X") # whatever symbol the player chooses
# create player 2
player2 = Player("Player2", "0") # whatever symbol the player chooses
which_player = player1
while not board.game_over:
space_taken, chosen_space = True, 0
while not space_taken:
# get next space from user
chosen_space = 3
space_taken = not board.is_space_taken(chosen_space)
if space_taken:
print("Sorry, this space is taken")
board.place_piece(chosen_space, which_player.symbol)
if board.game_over:
print (f'{which_player.symbol} wins!')
which_player = player1 if which_player is player2 else player2
Now, some specific comments on your code:
Comment your functions / methods: No matter how clear you think the meaning is, it is not. When you come back to your code 6-months from now, you will absolutely forget what you were trying to do. Comments will help.
Use type annotations for method parameters: Is index an integer, or a (x,y) tuple? Don't make users search to know this, make it obvious in the method call.
- Don't pass around '-1' to give meaningful information. Instead, define a constant "INVALID_ENTRY = -1". That should be the first and only time you see '-1' in your code. "If value == -1" means nothing to the reader. "If value == INVALID_ENTRY" does.