3
\$\begingroup\$

This is a simple TicTacToe game which allows 2 human players to play together. This involves no AI.

I am fairly new to programming and have only been learning python for 20 days. I need to know if the code which I have written is efficient in terms of approach, variable naming, commenting practices, typical conventions and new practices. Any suggestions for improvement are most welcome.

import random
#list ----> prints
#displays the moves_list in the form of a tic tac toe board
def displaygrid(moves_list):
    print("-------------")
    print("|{0:^3}|{1:^3}|{2:^3}|".format(moves_list[6],moves_list[7],moves_list[8]))
    print("-------------")
    print("|{0:^3}|{1:^3}|{2:^3}|".format(moves_list[3],moves_list[4],moves_list[5]))
    print("-------------")
    print("|{0:^3}|{1:^3}|{2:^3}|".format(moves_list[0],moves_list[1],moves_list[2]))
    print("-------------")

#no parameter ----> returns nothing
#keeps asking till the player gets ready
def are_you_ready():
    is_ready = 'n'
    while is_ready.lower() != 'y':
        is_ready = input("\nAre you ready to begin playing? (Y/N): ")
        if is_ready.lower() != 'y':
            print("No Problem! I will wait :)\n")

#no parameter ----> list
#asks the first player for the symbol [X/O] it wants to use
#and then returns a list with player 1 symbol at list[0]
def receive_signs():
    sign = input("Player 1: Will you play with 'X' or 'O'?: ")
    if sign.lower() == 'x':
        return ['X', 'O']
    elif sign.lower() == 'o':
        return ['O', 'X']

#int, list, string ----> int
#takes in the player no. [0/1], the moves_list and player symbol
#and returns the position where it wants to place the symbol.
#Note: Keeps asking for position till the player chooses an empty slot.
def take_input(player, moves_list, sign):
    while True:
        pos = int(input("Player {}: Choose an empty slot for your '{}' [1-9]: ".format(player, sign)))
        if 1 <= pos <= 9 and moves_list[pos-1] == '':
            break
    return pos

#list ----> boolean
#takes in the moves list and determines if 
#any of the player has won the game or not.
def somebody_wins(moves_list):
    if moves_list[0] == moves_list[1] == moves_list[2] and moves_list[0] != '':
        return True
    elif moves_list[3] == moves_list[4] == moves_list[5] and moves_list[3] != '':
        return True
    elif moves_list[6] == moves_list[7] == moves_list[8] and moves_list[6] != '':
        return True
    elif moves_list[0] == moves_list[3] == moves_list[6] and moves_list[0] != '':
        return True
    elif moves_list[1] == moves_list[4] == moves_list[7] and moves_list[1] != '':
        return True
    elif moves_list[2] == moves_list[5] == moves_list[8] and moves_list[2] != '':
        return True
    elif moves_list[0] == moves_list[4] == moves_list[8] and moves_list[0] != '':
        return True
    elif moves_list[2] == moves_list[4] == moves_list[6] and moves_list[2] != '':
        return True
    return False

#list, int ----> boolean
#determines a draw in the game
def draw(moves_list, turn_no):
    for item in moves_list:
        if item == '':
            return False
    return True

#int, string, list ----> list
#takes the desired position, symbol and the moves_list
#and places the players symbol on that position in the
#list and finnaly returns the new list
def modify_grid(pos, sign, moves_list):
    moves_list[pos-1] = sign
    return moves_list

#int ----> int
#determines the winner
def who_wins(turn_no):
    if turn_no % 2 == 0:
        return 1
    return 2


#flow of game
def mainFunction():
    play_more = 'y'
    while play_more.lower() == 'y':
        print("\n"*50)
        print("************************")
        print("*Welcome to Tic Tac Toe*")
        print("************************")
        #contains the status of moves
        moves_list = ['']*9

        #sign_list[0] will contain the symbol chosen by player 1
        #sign_list[1] will contain the symbol given to player 2
        sign_list = receive_signs()

        are_you_ready()
        print("\n"*100)
        turn_no = random.randint(0,1)
        print('Congratulations Player {}, You get to make the first turn'.format(turn_no+1))
        while not somebody_wins(moves_list)  and  not draw(moves_list, turn_no) :
            if(turn_no % 2 == 0):
                pos = take_input(1,moves_list, sign_list[0]) 
                moves_list = modify_grid(pos, sign_list[0], moves_list)
            else:
                pos = take_input(2,moves_list, sign_list[1])
                moves_list =  modify_grid(pos, sign_list[1], moves_list)
            print("\n"*100)
            displaygrid(moves_list)
            turn_no += 1
        print("\n" *100)
        displaygrid(moves_list)
        if somebody_wins(moves_list):
            wins = who_wins(turn_no-1)
            print("Congratulations! Player {} wins the game !".format(wins))
        else:
            print("Match Tied!")
            print("Well played both the players!")
        play_more = input(("Do you guys want to play again? [Y/N]: "))

mainFunction()
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

documentation

You document the methods with # above the code and specify the types of the arguments like this too. Python has a builtin way to annotate the type, and uses """ to delimit a docstring so:

#list ----> prints
#displays the moves_list in the form of a tic tac toe board
def displaygrid(moves_list):

would become

def displaygrid(moveslist: list) -> None:
    """displays the moves_list in the form of a tic tac toe board"""

naming

I find moves_list a bad name for 2 reasons. - you don't have to specify it is a lits in the variable name. The fact is is a plural already implies it is a collection - If you look at what it really is, it is not the list of the coordinates of the subsequent moves of the players, but it is the actual situation of the board in a flattened list, so why not call it board?

Boolean

you use 'y' and 'n' as boolean value. Better would be to use True and False

quit

There is no way to quit the game

DRY

Especially displaygrid and somebody_wins have a lot of repeated code. and can be more clearly expressed like this:

def somebody_wins(board):
    winning_combinations = {
        (0, 1, 2),
        (3, 4, 5),
        (6, 7, 8),
        (0, 3, 6),
        (1, 4, 7),
        (2, 5, 8),
        (0, 4, 8),
        (2, 4, 6),
    }
    return any(
        all(board[i] for i in combination) and board[combination[0]]
        for combination in winning_combinations
    )

def grouper(iterable, n, fillvalue=None):
    """
    Collect data into fixed-length chunks or blocks
    https://docs.python.org/3.5/library/itertools.html#itertools-recipes
    """
    # grouper('ABCDEFG', 3, 'x') --> ABC DEF Gxx"
    args = [iter(iterable)] * n
    return zip_longest(*args, fillvalue=fillvalue)

def displaygrid(board):
    rows = grouper(enumerate(board), 3)
    row_strings = (
        '|' + '|'.join(f'{sign:^3}' if sign else f'{place:^3}' for place, sign in row) + '|\n'
        for row in rows
    )
    hline = '-' * 13 + '\n'
    return hline + hline.join(row_strings) + hline

The code that does the logic can be changed from

if(turn_no % 2 == 0):
    pos = take_input(1,moves_list, sign_list[0]) 
    moves_list = modify_grid(pos, sign_list[0], moves_list)
else:
    pos = take_input(2,moves_list, sign_list[1])
    moves_list =  modify_grid(pos, sign_list[1], moves_list)

to:

player_info = {
    0: (1, sign_list[0]),
    1: (2, sign_list[1])
}
player_no, sign = player_info[turn_no % 2]
pos = take_input(player_no,moves_list, sign) 
moves_list = modify_grid(pos, sign, moves_list)

Miscellaneous

  • receive_signs returns None when an invalid character is passed in
  • There is no way to end a game prematurely. Perhaps add q as an extra option in take_input
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.