trjhtr

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 = "Userd".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='Playerp, 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()

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:

1. 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.




2. 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.



3. 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.