2
\$\begingroup\$

I'm still pretty new to programming, only been doing it a week. I'm just wondering if I made any glaring mistakes or anything that could be improved code wise?

import random

the_board = []
players = [{'type': 'player', 'piece': 'X'},
           {'type': 'player', 'level': 'smart', 'piece': 'O'}]


def print_board(board):
    for i in range(3):
        print(board[i * 3] + '|' + board[i * 3 + 1] + '|' + board[i * 3 + 2], end='')
        print('   ' + str(i * 3) + '|' + str(i * 3 + 1) + '|' + str(i * 3 + 2))
        if i < 2:
            print('-+-+-' + '   ' + '-+-+-')
    print()


def is_win(bo, player):
    # 0 1 2
    # 3 4 5
    # 6 7 8

    for i in range(3):
        return (bo[i * 3] == player and bo[i * 3 + 1] == player and bo[i * 3 + 2] == player) or \
               (bo[i] == player and bo[i+3] == player and bo[i+6] == player) or \
                (i <= 1 and (bo[i*2] == player and bo[4] == player and bo[8-(i*2)] == player))


def check_board(move, player):
    global the_board
    board_copy = the_board.copy()
    board_copy[move] = players[player]['piece']
    print('win')
    return is_win(board_copy, players[player]['piece'])


def ai():
    global players, the_board
    possible_moves = []
    final_move = None

    for i in range(9):
        if the_board[i] == ' ':
            possible_moves.append(i)

    if the_board[4] == ' ' and players[1]['level'] == 'smart':
        the_board[4] = players[1]['piece']
        return

    for i in range(len(possible_moves)):
        board_copy = the_board.copy()
        board_copy[possible_moves[i]] = players[1]['piece']
        if check_board(possible_moves[i], 1):
            the_board[possible_moves[i]] = players[1]['piece']
            print('win')
            return
        elif check_board(possible_moves[i], 0):
            print('lose')
            final_move = possible_moves[i]
        elif players[1]['level'] == 'smart' and final_move is None:
            if possible_moves[i] in [0, 2, 6, 8]:
                print('random')
                final_move = possible_moves[i]
    else:
        if final_move is not None:
            print('other')
            the_board[final_move] = players[1]['piece']
            return
        print('final')
        the_board[possible_moves[random.randint(0, len(possible_moves))]] = players[1]['piece']



def input_validation(option_type, options, choice):
    global the_board
    if option_type == 'number_range':
        if not str.isdigit(choice):
            print('It has to be a number')
            return False
        else:
            choice = int(choice)
        if choice < options[0] or choice > options[1]:
            print('You have to choose a number between 0 and 8')
            return False
        elif (the_board[choice] == 'X' or the_board[choice] == 'O'):
            print('There is already a move there, please try again.')
            return False
        else:
            return True

    if option_type == 'string':
        for i in range(len(options)):
            if choice == options[i]:
                return True
        else:
            print('That is not a valid option, your choices are:')
            for i in range(len(options)):
                print(options[i])
            else:
                print()
            return False


def player_options():
    valid = False
    global players

    while not valid:
        print('Do you want to play the cpu or player?')
        choice = input()
        choice = choice.lower()
        if input_validation('string', ['cpu', 'player'], choice):
            players[1]['type'] = choice
            break

    while not valid and players[1]['type'] == 'cpu':
        print('Do you want smart or dumb ai?:')
        choice = input()
        choice = choice.lower()
        if input_validation('string', ['smart', 'dumb'], choice):
            if choice == 'dumb':
                players[1]['level'] = choice
            break

    # while not valid:
    #     print('Player 1, do you want to be X or O?:')
    #     choice = input()
    #     choice = choice.upper()
    #     if input_validation('string', ['X', 'O'], choice):
    #         if choice == 'O':
    #             players[1]['piece'] = choice
    #         break


def game():
    move = 0
    turn = random.choice([True, False])

    for i in range(9):
        the_board.insert(i, ' ')

    for i in range(9):
        valid = False

        print_board(the_board)
        if is_win(the_board, players[turn]['piece']):
            print_board(the_board)
            print('Player ' + players[turn]['piece'] + ' is the winner!')
            return

        turn ^= True


        if turn == 0 or (players[1]['type'] == 'player' and turn == 1):
            while not valid:
                move = (input('Turn for ' + players[turn]['piece'] + '. Move on which space?: '))
                if input_validation('number_range', [0, 8], move):
                    move = int(move)
                    valid = True
                    the_board[move] = players[turn]['piece']
        elif players[1]['type'] == 'cpu' and turn == 1:
            ai()
    print_board(the_board)
    print('It is a tie!')

def retry():
    valid = False
    while not valid:
        print('Do you want to play again with the same settings? Y or N')
        choice = input()
        choice = choice.upper()
        if input_validation('string', ['Y', 'N'], choice):
            if choice == 'Y':
                game()
            else:
                player_options()
                game()


player_options()
game()
retry()
```
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Let's upgrade it point-to-point and get a better code at the end:

0. I tried to run your program and found several bad user-side behaviors:

U1. MANY unnecessary prints:

Do you want to play the cpu or player?
cpu
Do you want smart or dumb ai?:
smart
 | |    0|1|2
-+-+-   -+-+-
 | |    3|4|5
-+-+-   -+-+-
 | |    6|7|8

 | |    0|1|2
-+-+-   -+-+-
 |O|    3|4|5
-+-+-   -+-+-
 | |    6|7|8

Turn for X. Move on which space?: 4
There is already a move there, please try again.
Turn for X. Move on which space?: 5
 | |    0|1|2
-+-+-   -+-+-
 |O|X   3|4|5
-+-+-   -+-+-
 | |    6|7|8

win
win
random
win
win
win
win
win
win
win
win
win
win
win
win
other
O| |    0|1|2
-+-+-   -+-+-
 |O|X   3|4|5
-+-+-   -+-+-
 | |    6|7|8

Turn for X. Move on which space?:

Many of them are in ai() function:

    for i in range(len(possible_moves)):
        board_copy = the_board.copy()
        board_copy[possible_moves[i]] = players[1]['piece']
        if check_board(possible_moves[i], 1):
            the_board[possible_moves[i]] = players[1]['piece']
            print('win')
            return
        elif check_board(possible_moves[i], 0):
            print('lose')
            final_move = possible_moves[i]
        elif players[1]['level'] == 'smart' and final_move is None:
            if possible_moves[i] in [0, 2, 6, 8]:
                print('random')
                final_move = possible_moves[i]

We will remove them. If you want, you can import logging module and log them to debug stream with logging.debug(...).

U2. You can't just manually quit the game. So let's modify retry function:

def retry():
    valid = False
    while not valid:
        print('Do you want to play again with the same settings? Y or N (Q to exit)')
        choice = input()
        choice = choice.upper()
        if input_validation('string', ['Y', 'N', 'Q'], choice):
            if choice == 'Y':
                game()
            elif choice == 'Q':
                return
            else:
                player_options()
                game()

U3. The game randomly crashes and doesn't react to winning positions.

Look at your is_win() code:

def is_win(bo, player):
    # 0 1 2
    # 3 4 5
    # 6 7 8

    for i in range(3):
        return (

return statement immediately exits from the function with a value you wrote to return. So this code will work only for i == 0. If you want to check everything and return if there is True anywhere, you should modify your code like this (and, please, use consistent variables! If you are using board, use it everywhere for boards. Don't use board, bo and smth like b in different places):

def is_win(board, player):
    # 0 1 2
    # 3 4 5
    # 6 7 8

    for i in range(3):
        if (
            (board[i * 3] == player and
             board[i * 3 + 1] == player and
             board[i * 3 + 2] == player)
            or
            (board[i] == player and
             board[i+3] == player and
             board[i+6] == player)
            or
            (i <= 1 and
             (board[i*2] == player and
              board[4] == player and
              board[8-(i*2)] == player))
        ):
            return True
    return False

Now we are going to the code directly:

  1. Avoid global variables! In 99.99% you don't need them and can easily work without them. Global variables can (and often will) lead you to very hard-to-debug errors. We will eliminate global variables by creating TicTac class and converting all functions to class functions (very much code, you will see it later).

  2. DRY (Don't Repeat Yourself). Everywhere in your code you have variables like players[1]['piece'] and players[1]['level']. You can create new short variable (especially because we created a class) and use it everywhere. If you will change your player structure (for 3rd player, for example), you will not have to change EVERYTHING in your code. We will replace them with self.ai_level and self.ai_X

  3. In ai() function you have a line:

self.the_board[possible_moves[random.randint(0, len(possible_moves))]] = self.ai_X

random.randint can return bounds (e.g. random.randint(0, 5) can return both 0 or 5) so this line sometimes raises an error (when you are trying to get the last element of random.randint). You can define right bound as len(possible_moves) - 1 or you can just use random.choice:

self.the_board[random.choice(possible_moves)] = self.ai_X

  1. Some small improvements like:
for i in range(9):
    self.the_board.insert(i, ' ')

to

self.the_board = [' '] * 9

and like it.

So we have a final code. Honestly, it is better to re-write the whole game from scratch but you will realize it only some years later :) This code still not ideal and has many places to improve but it is better than it was.

import random

class TicTac(object):
    def __init__(self):
        self.the_board = [' '] * 9
        self.players = [
            {'type': 'player', 'piece': 'X'},
            {'type': 'player', 'level': 'smart', 'piece': 'O'}
        ]
        self.ai_level = None
        self.ai_X = None

    def print_board(self, board):
        for i in range(3):
            print(board[i * 3] + '|' + board[i * 3 + 1] + '|' + board[i * 3 + 2], end='')
            print('   ' + str(i * 3) + '|' + str(i * 3 + 1) + '|' + str(i * 3 + 2))
            if i < 2:
                print('-+-+-' + '   ' + '-+-+-')
        print()

    def is_win(self, board, player):
        # 0 1 2
        # 3 4 5
        # 6 7 8

        for i in range(3):
            if (
                (board[i * 3] == player and
                 board[i * 3 + 1] == player and
                 board[i * 3 + 2] == player)
                or
                (board[i] == player and
                 board[i+3] == player and
                 board[i+6] == player)
                or
                (i <= 1 and
                 (board[i*2] == player and
                  board[4] == player and
                  board[8-(i*2)] == player))
            ):
                return True
        return False

    def check_board(self, move, player):
        board_copy = self.the_board.copy()
        board_copy[move] = self.players[player]['piece']
        return self.is_win(board_copy, self.players[player]['piece'])

    def ai(self):
        possible_moves = []
        final_move = None

        for i in range(9):
            if self.the_board[i] == ' ':
                possible_moves.append(i)

        if self.the_board[4] == ' ' and self.ai_level == 'smart':
            self.the_board[4] = self.ai_X
            return

        for i in range(len(possible_moves)):
            board_copy = the_board.copy()
            board_copy[possible_moves[i]] = self.ai_X
            if self.check_board(possible_moves[i], 1):
                self.the_board[possible_moves[i]] = self.ai_X
                return
            elif self.check_board(possible_moves[i], 0):
                final_move = possible_moves[i]
            elif self.ai_level == 'smart' and final_move is None:
                if possible_moves[i] in [0, 2, 6, 8]:
                    final_move = possible_moves[i]
        else:
            if final_move is not None:
                self.the_board[final_move] = self.ai_X
                return
            self.the_board[random.choice(possible_moves)] = self.ai_X

    def input_validation(self, option_type, options, choice):
        if option_type == 'number_range':
            if not str.isdigit(choice):
                print('It has to be a number')
                return False
            else:
                choice = int(choice)
            if choice < options[0] or choice > options[1]:
                print('You have to choose a number between 0 and 8')
                return False
            elif (self.the_board[choice] == 'X' or self.the_board[choice] == 'O'):
                print('There is already a move there, please try again.')
                return False
            else:
                return True

        if option_type == 'string':
            for i in range(len(options)):
                if choice == options[i]:
                    return True
            else:
                print('That is not a valid option, your choices are:')
                for i in range(len(options)):
                    print(options[i])
                else:
                    print()
                return False

    def player_options(self):
        valid = False

        while not valid:
            print('Do you want to play the cpu or player?')
            choice = input()
            choice = choice.lower()
            if self.input_validation('string', ['cpu', 'player'], choice):
                self.players[1]['type'] = choice
                break

        while not valid and self.players[1]['type'] == 'cpu':
            self.ai_X = self.players[1]['piece']
            print('Do you want smart or dumb ai?:')
            choice = input()
            choice = choice.lower()
            if self.input_validation('string', ['smart', 'dumb'], choice):
                if choice == 'dumb':
                    self.players[1]['level'] = choice
                    self.ai_level = choice
                break

    def game(self):
        move = 0
        turn = random.choice([True, False])

        for i in range(9):
            valid = False

            self.print_board(self.the_board)
            if is_win(self.the_board, self.players[0]['piece']) or \
                    is_win(self.the_board, self.players[1]['piece']):
                self.print_board(self.the_board)
                print('Player ' + self.players[turn]['piece'] + ' is the winner!')
                return

            turn ^= True

            if turn == 0 or (self.players[1]['type'] == 'player' and turn == 1):
                while not valid:
                    move = (input('Turn for ' + self.players[turn]['piece'] + '. Move on which space?: '))
                    if self.input_validation('number_range', [0, 8], move):
                        move = int(move)
                        valid = True
                        self.the_board[move] = self.players[turn]['piece']
            elif self.players[1]['type'] == 'cpu' and turn == 1:
                self.ai()
        print('It is a tie!')

    def retry(self):
        valid = False
        while not valid:
            print('Do you want to play again with the same settings? Y or N (Q to exit)')
            choice = input()
            choice = choice.upper()
            if input_validation('string', ['Y', 'N', 'Q'], choice):
                if choice == 'Y':
                    self.game()
                elif choice == 'Q':
                    return
                else:
                    self.player_options()
                    self.game()

game = TicTac()
game.player_options()
game.game()
game.retry()
\$\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.