1
\$\begingroup\$

I have created a simple Tic-Tac-Toe program. Can someone please review it and let me know where I can improve.

Here we can play against computer. Program start by putting 'X' at the center that is at the position 5. Then ask user to enter number from available position and then assign 'O' on selected position. There are four possible verdicts: the game should continue, or the game ends with a tie, your win, or the computer's win;

import random

def board():
    "Creating Board dictionary"
    dic_board = {}
    v = 1
    for i in range(3):
        for j in range(3):
            k = str(i) + str(j)
            dic_board[k] = v
            v += 1
    update_board(dic_board)

def draw_board(board):
    """Draw board"""
    for i in range(3):
        print('+' + '-' * 5 + '+' + '-' * 5 + '+' + '-' * 5 + '+', end='')
        print()
        print('|' + ' ' * 5 + '|' + ' ' * 5 + '|' + ' ' * 5 + '|')
        for j in range(3):
            k = str(i) + str(j)
            print('| ' + str(board[k]) + ' ' * 3, end='')
        print('|')
        print('|' + ' ' * 5 + '|' + ' ' * 5 + '|' + ' ' * 5 + '|')
    print('+' + '-' * 5 + '+' + '-' * 5 + '+' + '-' * 5 + '+', end='')
    print()

def update_board(board):
    """the first move belongs to the computer -
    it always puts its first 'X' in the middle of the board that is at 5;"""
    total_moves = 0
    flag = True  #Flag = True for computer's move and False for user's move
    while total_moves < 9 :
        available_num = [v for k, v in board.items() if v not in ('X','O')]
        if flag :
            if total_moves == 0:
                update_moves(board, 'Comp', 5)
                flag = False
                sign = 'X'
                draw_board(board)
            else:
                num = random.choice(available_num)
                update_moves(board, 'Comp', num)
                flag = False
                sign = 'X'
                draw_board(board)
        else:
            try:
                s = str(available_num)
                val = int(input('Select Number '+ s + ': '))
                update_moves(board, 'User', val)
                flag = True
                sign = 'O'
            except:
                print('Invalid Input!!')
                continue
        total_moves += 1

        v = victory(board,sign)

        if v:
            print(v)
            if sign == 'O' :
                draw_board(board)
            break
    else :
        print('Tie !!')
        print('Game Over!!')




def update_moves(board, moves,val):
    "Updating borad as per input"
    key_to_update = [k for k, v in board.items() if v == val]
    if moves == 'Comp':
        board[key_to_update[0]] = 'X'
    elif moves == 'User':
        board[key_to_update[0]] = 'O'
    return board

def victory(board,sign):
    message = ''
    message = row_status(board,sign,message)
    message = column_status(board,sign,message)
    message = digonals_status(board,sign,message)
    return message



def row_status(board,sign,message):
    """checking for Horizontal row"""
    if board['00'] == board['01'] == board['02'] == sign or \
            board['10'] == board['11'] == board['12'] == sign or \
            board['20'] == board['21'] == board['22'] == sign:
        if sign == 'X':
            message = 'computer win !!'
        elif sign == 'O':
            message = 'Your win !!'
    return message

def column_status(board,sign,message):
    """checking for verticle column """
    if board['00'] == board['10'] == board['20'] == sign or \
            board['01'] == board['11'] == board['21'] == sign or \
            board['02'] == board['12'] == board['22'] == sign:
        if sign == 'X':
                message = 'computer win !!'
        elif sign == 'O':
            message = 'Your win !!'
        return message
    
def digonals_status(board,sign,message):
    """checking for digonals """
    if board['00'] == board['11'] == board['22'] == sign or \
       board['02'] == board['11'] == board['20'] == sign :
        if sign == 'X':
            message = 'computer win !!'
        elif sign == 'O':
            message = 'Your win !!'
    return message

print(board())
\$\endgroup\$
1
  • 1
    \$\begingroup\$ I have just played a game where I won by getting 3 in a row across the top row. This did not register as a win, and the computer played 3 in a row across the bottom row, which still hasn't registered a win. The final result is a tie despite 2 winning positions. That's worth looking into. \$\endgroup\$
    – Kraigolas
    Jun 10, 2021 at 2:08

1 Answer 1

1
\$\begingroup\$

Print Statements

If you write print(...), that includes a newline character. In your draw_board function, you have a couple lines where you specifically tell Python you don't want a newline, and then on the very next line you call print with no argument to get a newline. You should just remove the end='' in those cases:

def draw_board(board):
    """Draw board"""
    for i in range(3):
        print('+' + '-' * 5 + '+' + '-' * 5 + '+' + '-' * 5 + '+')
        print('|' + ' ' * 5 + '|' + ' ' * 5 + '|' + ' ' * 5 + '|')
        for j in range(3):
            k = str(i) + str(j)
            print('| ' + str(board[k]) + ' ' * 3, end='')
        print('|')
        print('|' + ' ' * 5 + '|' + ' ' * 5 + '|' + ' ' * 5 + '|')
    print('+' + '-' * 5 + '+' + '-' * 5 + '+' + '-' * 5 + '+')

Choice of Dictionary

I won't actually be providing the code for this change, because it would require rewriting a lot. However, look at how you index your dictionary:

board['10']

This means board row 1, column 0. This is identical to just using nested lists:

board[1][0]

So your board would probably be better off as nested lists:

def board():
    "Creating Board"
    board = [[3 * row + col for col in range(1, 4)] for row in range(3)]
    # [[1, 2, 3], [4, 5, 6], [7, 8, 9]]

This is easier to wrap your head around in the context of the problem.

Main Statement

In your current code, you actually handle the processing of the game from board, a function which declares in its docstring to be a function that "creates the game board dictionary". This is bad practice. A function should do what it claims to do. Beyond that, you should never have a function which serves as both a supporting function and the showrunner. Do this instead:

def board():
    "Creating Board dictionary"
    dic_board = {}
    v = 1
    for i in range(3):
        for j in range(3):
            k = str(i) + str(j)
            dic_board[k] = v
            v += 1
    return dic_board

...

if __name__ == "__main__":
    update_board(board())

This might look like a small change but it makes a big difference. In fact, using my suggested changes to board, you could just instantiate the board in the main block as well and remove the definition for board altogether. The block contained in

if __name__ == "__main__":

runs if and only this file specifically was run, which allows you to import all other functions that you've defined here into another module without running your game. It also adds some readability: anyone reading your code can go straight to this code block and understand that this is the program being run.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you. I have considered your suggestion and posted new draft of Tic Tac Toe. \$\endgroup\$
    – ch2019
    Jun 10, 2021 at 20:40

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.