5
\$\begingroup\$

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()
\$\endgroup\$
  • \$\begingroup\$ I mean it can be scaled to NXN board. Sorry for poor vocabulary. \$\endgroup\$ – plotop May 3 '18 at 7:29
2
\$\begingroup\$

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.
\$\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.