1
\$\begingroup\$

I have implemented suggestion and created new draft of the Tic Tac Toe program. Please review and let me know where I can improve.

import random

def update_board():
    """Play game"""
    board = [[3 * row + col for col in range(1, 4)] for row in range(3)]

    total_count = 0
    UserTurn = False

    while total_count < 9:
        available_number = remaining_num(board)
        message = ''
        if total_count == 0 :
            board[1][1] = 'X'
            UserTurn = True
        elif UserTurn == False :
            #Computer's Turn
            num = random.choice(available_number)
            num_indx = find_index(num, board)
            board[num_indx[0][0]][num_indx[0][1]] = 'X'
            UserTurn = True
        elif UserTurn == True :
            #User's Turn
            UserTurn = False
            num = int(input(f'select number {available_number}: '))
            num_indx = find_index(num, board)
            if num_indx:
                board[num_indx[0][0]][num_indx[0][1]] = 'O'
            else :
                print('Selected number is invalid!')
        total_count += 1
        message = check(board, message)
        if message:
            draw_board(board)
            print(message)
            break
        elif message == '' and len(available_number) <= 1:
            draw_board(board)
            print('Tie !!')
            break
        draw_board(board)


def find_index(num,board):
    """Find index of the number"""
    num_indx = [[indx1, indx2] for indx1, val1 in enumerate(board)
                for indx2, val2 in enumerate(val1)
                if val2 == num]
    return num_indx

def remaining_num (board):
   """Available number list."""
   avail_num = [board[i][j] \
                for i in range(len(board)) \
                for j in range(len(board)) \
                if board[i][j] not in ('X', 'O')]
   return avail_num



def check(board,message):
    """Ckeck for win."""
    if board[0][0] == board[0][1] == board[0][2] == 'X' or \
       board[1][0] == board[1][1] == board[1][2] == 'X' or \
       board[2][0] == board[2][1] == board[2][2] == 'X' or \
       board[0][0] == board[1][0] == board[2][0] == 'X' or \
       board[0][1] == board[1][1] == board[2][1] == 'X' or \
       board[0][2] == board[1][2] == board[2][2] == 'X' or \
       board[0][0] == board[1][1] == board[2][2] == 'X' or \
       board[0][2] == board[1][1] == board[2][0] == 'X' :
                message = 'Computer win !!'
    elif board[0][0] == board[0][1] == board[0][2] == 'O' or \
         board[1][0] == board[1][1] == board[1][2] == 'O' or \
         board[2][0] == board[2][1] == board[2][2] == 'O' or \
         board[0][0] == board[1][0] == board[2][0] == 'O' or \
         board[0][1] == board[1][1] == board[2][1] == 'O' or \
         board[0][2] == board[1][2] == board[2][2] == 'O' or \
         board[0][0] == board[1][1] == board[2][2] == 'O' or \
         board[0][2] == board[1][1] == board[2][0] == 'O' :
                message = 'User win !!'
    return message

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



if __name__ == "__main__":
    update_board()
\$\endgroup\$
1
\$\begingroup\$

Follow PEP8 naming conventions when naming variables and functions.


user_turn == False should be user_turn is False, or even better not user_turn. user_turn == True should simply be user_turn, since bool_var == (bool_var == True).


check

Why do you pass message as an argument, if you just overwrite it? Seems like you actually want a default argument: def check(board, message='').

That amount of code duplication should always raise a red flag. Writing these conditions out manually is error-prone and hurts maintainability / scalability. You could take some inspiration from a previous answer of mine (which I'm sure can be further improved) or some other threads on CodeReview.


draw_board

Pulling out separator and empty rows could already make this a whole lot more readable and maintainable. This is an improved version, not yet perfect by any means:

def build_row(sep, fill):
    return f"{sep}{sep.join([fill * 5] * 3)}{sep}"


def draw_board(board):
    sep_row = build_row(sep='+', fill='-')
    empty_row = build_row(sep='|', fill=' ')

    for i in range(3):
        print(sep_row)
        print(empty_row)

        for j in range(3):
            print('| ' + str(board[i][j]) + ' ' * 3, end='')

        print('|')
        print(empty_row)

    print(sep_row)

General approach

Your approach using find_index, remaining_num seems overcomplicated in some places. You could simply keep a list of the remaining numbers and remove each number once a symbol is placed on it.

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