9
\$\begingroup\$

Here is my version of TicTacToe on python 3.x

I'm learning python for a several weeks and I will be very appretiated if you take a look on my script and review it!

P.S. I've used NumPad to input move to make game more user-friendly. So 5 means center, 7 means right-upper corner, 6 means right side etc.

from random import randint, choice
from os import system as bash
from time import time


def intInput(StringToDisplay):
    # Simply checks that input is valid integer
    while True:
        try:
            x = int(input(StringToDisplay))
            return x
            break
        except ValueError:
            print('Input integer number, please')
        except Exception:
            print('Unexpected error or keyboard interrupt')


def drawBoard():
    print('\
 ╔═══╦═══╦═══╗\n\
 ║ {0} ║ {1} ║ {2} ║\n\
 ╠═══╬═══╬═══╣\n\
 ║ {3} ║ {4} ║ {5} ║\n\
 ╠═══╬═══╬═══╣\n\
 ║ {6} ║ {7} ║ {8} ║\n\
 ╚═══╩═══╩═══╝ '.format(
               board_status[7], board_status[8], board_status[9],
               board_status[4], board_status[5], board_status[6],     
               board_status[1], board_status[2], board_status[3]))


def askPlayerLetter():
    # Function that asks which letter player wants to use
    print('Do you want to be X or O?')
    Letter = input().upper()
    while Letter != 'X' and Letter != 'O':
        print('Please type appropriate symbol')
        Letter = input('Prompt: ').upper()
    if Letter == 'X':  # then X will be used by player; O by computer
        return ['X', 'O']
    else:
        return ['O', 'X']


def whoGoesFirst():
    # Timer used to count 0.75 seconds while displaying who goes first
    if randint(0, 1) == 0:
        CurrentTime, Timer = time(), time() + 0.75
        print('You go first')
        while Timer > CurrentTime:
            CurrentTime = time()
        return 'player'
    else:
        CurrentTime, Timer = time(), time() + 0.75
        print('Computer goes first')
        while Timer > CurrentTime:
            CurrentTime = time()
        return 'computer'


def makeMove(Board, Move, Letter):
    Board[Move] = Letter


def isSpaceFree(Board, Move):
    return Board[Move] == ' '


def playerMove():
    Move = 0
    while not (0 < Move < 10) or not (isSpaceFree(board_status, int(Move))):
        Move = intInput('Enter your move: ')
    return int(Move)


def isWinner(brd, lttr):
    # Returns a boolean value. brd (board) and lttr (letter) used to make
    # code block compact.
    return ((brd[7] == lttr and brd[8] == lttr and brd[9] == lttr) or
            (brd[4] == lttr and brd[5] == lttr and brd[6] == lttr) or
            (brd[1] == lttr and brd[2] == lttr and brd[3] == lttr) or
            (brd[7] == lttr and brd[5] == lttr and brd[3] == lttr) or
            (brd[9] == lttr and brd[5] == lttr and brd[1] == lttr) or
            (brd[7] == lttr and brd[4] == lttr and brd[1] == lttr) or
            (brd[8] == lttr and brd[5] == lttr and brd[2] == lttr) or
            (brd[9] == lttr and brd[6] == lttr and brd[3] == lttr))


def computerMove():
    '''
    Simple AI that checks
    1)Can computer win in the next move
    2)Can player win in the next move
    3)Is there any free corner
    4)Is center is free
    5)Is there any free side
    And returns a move digit

    '''


    for i in range(1, 10):
        Copy = board_status.copy()
        if isSpaceFree(Copy, i):
            makeMove(Copy, i, ComputerLetter)
            if isWinner(Copy, ComputerLetter):
                return i

    for i in range(1, 10):
        Copy = board_status.copy()
        if isSpaceFree(Copy, i):
            makeMove(Copy, i, PlayerLetter)
            if isWinner(Copy, PlayerLetter):
                return i

    move = randomMoveFromList([7, 9, 1, 3])
    if move is not None:
        return move

    if isSpaceFree(board_status, 5):
        return 5

    move = randomMoveFromList([8, 4, 2, 6])
    if move is not None:
        return move


def randomMoveFromList(MovesList):
    PossibleMoves = []
    for i in MovesList:
        if isSpaceFree(board_status, i):
            PossibleMoves.append(i)
    if len(PossibleMoves) != 0:
        return choice(PossibleMoves)
    else:
        return None


def isBoardFull():
    for i in range(1, 10):
        if isSpaceFree(board_status, i):
            return False
    return True


def playAgain():
    print('Do you want to play again? [y/N]')
    PlayAgainInput = input().lower()
    return (PlayAgainInput.startswith('y') or PlayAgainInput == '')

# "bash('clear')" function simply clears the screen of the terminal.
# If you want run this script on system that uses other shell then
# substitute "clear" with a command that your shell uses to clear the screen
# P.S. for windows it is "cls".

bash('clear')
print('Welcome to Tic Tac Toe')
PlayAgainWish = True
print('To win, you have to place 3 X-s or O-s in a row.\n\
Use NumPad to enter your move (!). Here is the key map.')
board_status = ['', 1, 2, 3, 4, 5, 6, 7, 8, 9]
drawBoard()
print('You have to be sure that you are making move to a free cell.\n\n')
PlayerLetter, ComputerLetter = askPlayerLetter()
while PlayAgainWish:
    bash('clear')
    board_status = 10 * [' ']
    turn = whoGoesFirst()
    while True:
        if turn == 'player':
            bash('clear')
            print('   YOUR MOVE')
            drawBoard()
            move = playerMove()
            makeMove(board_status, move, PlayerLetter)
            turn = 'computer'
            if isWinner(board_status, PlayerLetter):
                bash('clear')
                print('Hooray, you have won the game!')
                drawBoard()
                PlayAgainWish = playAgain()
                break
            elif isBoardFull():
                bash('clear')
                print("It's a tie!")
                drawBoard()
                PlayAgainWish = playAgain()
                break
        else:
            # All this dots and timers are used to make animation of
            # computer moving. You will understand if you will run the script.
            for i in ['', '.', '..', '...']:
                bash('clear')
                print(' Computer is making move' + i)
                drawBoard()
                CurrentTime, Timer = time(), time() + 0.15
                while Timer > CurrentTime:
                    CurrentTime = time()
                if i == '..':
                    move = computerMove()
                    makeMove(board_status, move, ComputerLetter)
                    turn = 'player'
            if isWinner(board_status, ComputerLetter):
                bash('clear')
                print('Oops, you lose!')
                drawBoard()
                PlayAgainWish = playAgain()
                break
            elif isBoardFull():
                bash('clear')
                print("It's a tie!")
                DrawBoard()
                PlayAgainWish = playAgain()
                break
\$\endgroup\$
1
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ Jan 7, 2017 at 9:19

2 Answers 2

4
\$\begingroup\$
  • First of all, nice ingenuity on displaying an almost graphical board, that's good for a beginner.

  • The intInput function should follow PEP8 and be something like 'get_int_input'. Also, since you want the number to be between 1 and 9, you can add optional parameters to make the function more generic and decouple the check. StringToDisplay should also be more like displayed_prompt. The same goes for all the variable/function names.

  • Also in intInput you have a break after the return which is useless, since you're going to return anyway. You also don't need a variable for the input, since the exception is going to be caught anyway.

Like this:

def get_int_input(displayed_prompt='', min_value=1, max_value=9):
    while True:
        try:
            input_value = int(input(displayed_prompt))
            if (input_value >= min_value and input_value <= max_value):
                return input_value
            else:
                raise ValueError
        except ValueError:
            print('Input an integer number between {0} and {1}, please'.format(min_value, max_value))
        except Exception:
            print('Unexpected error or keyboard interrupt')
  • In askPlayerLetter you don't need to print and then input, you can input with a prompt. Also, you can use an array or tuple for the allowed values.

Like this:

def ask_player_letter():
    allowed_letters = ['X', 'O']
    letter = input('Do you want to be X or O?').upper()
    while letter not in allowed_letters:
        letter = input('Please choose between {} and {}: '.format(
            *allowed_letters)).upper()
    if letter == 'X':
        return allowed_letters
    else:
        return allowed_letters[::-1]
  • You have a lot of numbers, like 0.75 that should be defined somewhere in a variable, so you can fine-tune what you need in one place instead of hunting around the code for places where you use those values.

  • You use a copy of your board to make the move and then check the winning condition. That's all good and fine because this is a tiny program, but you should keep scalability in mind. A copy is an expensive operation, so if some day you want to optimize you can keep two "boards" in memory and doing/undoing the operation in the "off-screen" board and putting it in the "on-screen" board if that's the move you want. That way you'll only set/unset one element at a time instead of copying back everything every cycle of the loop.

  • In randomMoveFromList you can use a list comprehension.

Like this:

def random_move_from_list(moves_list):
    PossibleMoves = [move for move in moves_list if isSpaceFree(board_status, move)]
    if len(PossibleMoves) != 0:
        return choice(PossibleMoves)

    return None
  • You could consider using a generator, so that you could have a function like get_next_move and yield a value, but in this specific case that would probably be a bit of overkill.

  • In isBoardFull you go through the board just to count how many cells are occupied. Since your code is the only thing occupying the cells, you can keep a counter every time a cell is occupied and use that, avoiding the loop.

  • To check victory you could think about some tricks. For example, you could still show the X and the O, but internally you could save them as -1 and 1, so if the sum of one of the rows or columns is -3 or 3, someone has won. Keep in mind that you can do something like if (sum(brd[4:6]) in [-3, 3]).

\$\endgroup\$
5
  • \$\begingroup\$ I appreciate the immense effort you put into this response. This was actually really helpful, even though I don't fully understand everything just yet (especially generators, but I can go look it up). \$\endgroup\$
    – Akyshnik
    Jan 7, 2017 at 6:42
  • \$\begingroup\$ I guessed that generators were a bit advanced for a beginner, that's why I didn't include an example for those. But for the rest, if you have questions just ask :-) \$\endgroup\$
    – ChatterOne
    Jan 7, 2017 at 7:50
  • \$\begingroup\$ You said internally you could save them as -1 and 1, so if the sum of one of the rows or columns is -3 or 3, someone has won. I think that would change game mechanics way too much so I have to design a new one, so I decided check a slice of board_status for equality so I ended up with this code (I substituted isWinner function in the OP post.) So what do you think about it? \$\endgroup\$
    – Akyshnik
    Jan 7, 2017 at 9:15
  • \$\begingroup\$ Oh, it is forbidden to edit my code, so i upload new function to Pastebin \$\endgroup\$
    – Akyshnik
    Jan 7, 2017 at 10:26
  • 1
    \$\begingroup\$ @Akyshnik, you can post a new question with the new code, and link back to this question for context. \$\endgroup\$ Jan 7, 2017 at 11:39
3
\$\begingroup\$

Few observations:

  1. Putting in \ns everywhere makes your code look somewhat cluttered. Instead, use Python's support for raw strings, in the form r"...". Note the r prefix before the double quotes. That's what designates the string as a raw string. In raw strings, each character is stored WYSIWYG, which means that linebreaks in the string are preserved.

  2. Avoid the burden on the user of the script for choosing the correct clear-screen command. It is possible to detect the OS in Python via:

    >>> import os
    >>> os.name
    

    This will result in 'nt' for Windows and 'posix' for a POSIX system like any Unix derivative, including Linux.

    Using the above data, you can decide whether to use system('clear') or system('cls'). You might then consider renaming the import to something other than bash, or not renaming it at all (like I showed).

  3. Reduce the number of unexplained "magic numbers" in your code. You hardcode 10 for stuff like the number of rows or columns, etc. Replace these with declared constants or function parameters, and follow the PEP8 naming conventions.

  4. You hardcode a lot of checks, preventing scalability to larger grids. Using loops and boolean accumulators for checking conditions, especially for isWinner() can be helpful for this purpose. Loops can also be used to generate the necessary format strings for output.

  5. Maybe you should include some sort of command-line help (especially about how to play)?

Style

These are recommendations based on PEP8, the Python style guide.

  1. For triple-quoted strings, always use double quote characters to be consistent with the docstring convention in PEP 257 .

  2. In Python, the accepted style is to use names that start with capital letters as class and type names, hence variables should be:

    Method Names and Instance Variables

    Use the function naming rules: lowercase with words separated by underscores as necessary to improve readability.

\$\endgroup\$
1
  • \$\begingroup\$ Hey there, thanks for your reply. I will change my code according to your suggestions! \$\endgroup\$
    – Akyshnik
    Jan 7, 2017 at 6:45

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.