3
\$\begingroup\$

Tic-Tac-Toe from scratch. Code Ugliness check before I implement the win-condition and maybe get set in to bad habits. Questions:

As implemented:

Better to pass board between functions or set as a global variable?

OK to use same variable name within different functions?

OOP:

Would you recommend using Object Orientated Programming for this?

import sys
import copy
import random
#this function accepts a user input if it's contained in a list
def get_user_input(input_array,prompt):
    valid_input = False

    #the purpose of this is to convert a range() to list
    acceptable_input_array = list(copy.copy(input_array))

    for i,element in enumerate(acceptable_input_array):
        #convert elements in array to string
        acceptable_input_array[i] = str(element).upper()
        
    while not valid_input:
        print("Please enter choice of", 
        acceptable_input_array,
        "for",
        prompt,
        ", blank to cancel.")

        user_input = input()
        user_input = user_input.upper()
        #if blank input
        if user_input == '':
            #exit program
            print('Exiting program.')
            sys.exit()
        #if input is in acceptable input
        if(user_input in acceptable_input_array):
            valid_input = True
        else:
            print('Incorrect Input')
    return user_input

#this function simply prints the board state
def print_board(board):
    #0 is blank, 1 is X, 2 is O
    token = ''
    for i, element in enumerate(board): 
        
        if element == 1:
            token = 'X'
        elif element == 2:
            token = 'O'
        else:
            token = ' '
        
        position = i%BOARD_SIZE
        #position = 0 is leftmost
        if position == 0:
            #print numberals at left of table
            print(i//BOARD_SIZE,end='')
        #.center to make an equidistant board
        print(token.center(BOARD_SIZE), end='')
        print('|',end='')
        #position BOARD_SIZE - 1 is rightmost
        if position == BOARD_SIZE - 1:
            #print newline
            print()
            #each token has BOARD_SIZE + BOARD_SIZE^2 spacing
            print('-' * (BOARD_SIZE + BOARD_SIZE**2))

    print(' ',end='')
    for j in range(BOARD_SIZE):
        #print numerals at bottom of table
        print(str(j).center(BOARD_SIZE+1),end='')

    print()

#this function accepts co-ordinates from player then updates board
def player_turn(board):
    valid_coords = False
    while not valid_coords:
        i = int(get_user_input(range(BOARD_SIZE),'i co-ordinate'))
        j = int(get_user_input(range(BOARD_SIZE),'j co-ordinate'))
        print('You selected position [', i, j, ']')
        index = j*BOARD_SIZE + i
        #if not already X or O in position
        if(board[index] == 0):
            valid_coords = True
        else:
            print('Spot already taken, please enter valid co-ordiantes.')

    board[index] = 1
    print_board(board)

#this function chooses a random free space for a.i. turn
def cpu_turn(board):
    print('cpu turn')
    list_of_indices = []
    for i, element in enumerate(board):
        if(element == 0):
            list_of_indices.append(i)
    print(list_of_indices)
    random_index = random.choice(list_of_indices)
    print('random index: ', random_index)
    board[random_index] = 2
    print_board(board)


#variable board size, based on user input.
BOARD_SIZE = int(get_user_input([3,5,7],'board size'))

#create empty list board with length of BOARD_SIZE squared
#board represented by a 1D array
board = [0] * BOARD_SIZE**2

print_board(board)
print("You are playing as 'X'")
while True:
    player_turn(board)
    cpu_turn(board)
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Answers to the explicit questions

Better to pass board between functions or set as a global variable?

I'd recommend against global variables, since accessing those will lead to side-effects within your functions. Either pass the object to pure functions or create an instance of the board as a property of some superordinate Game class.

OK to use same variable name within different functions?

This is perfectly fine.

Would you recommend using Object Orientated Programming for this?

Programming paradigms should be selected based on the problem to solve. Python is a multi-paradigm language. You can use functional or OO programming and mix those. You should use the paradigm that most fittingly solves your problem at hand. That being said, your functional approach is absolutely reasonable.

General review

PEP 8

Read and apply PEP8. Your code especially can be improved by applying the recommended whitespace rules.

Do not translate values

You use 0, 1 and 2 for values on the board. Later you replace them with ' ', 'X'and 'O'. Why not use those values directly? Even better, you can use an enum.Enum for those values:

from enum import Enum


class CellValue(Enum):
    """Cell values."""

    EMPTY = ' '
    PLAYER = 'X'
    CPU = 'O'

Suboptimal code

This:

...

#the purpose of this is to convert a range() to list
acceptable_input_array = list(copy.copy(input_array))

for i,element in enumerate(acceptable_input_array):
    acceptable_input_array[i] = str(element).upper()

can be siplified to this:

acceptable_input = [str(element) for element in input_array]

And since you use it for a membership check, you can use a set instead of a list:

acceptable_input = {str(element) for element in input_array}

Since all elements of the range are ints anyway, calling str.upper() on their string representation is pretty useless.
Pro tip: If you want to compare strings case-insensitively, use str.casefold().

Use early return in loops

Instead of using a boolean loop variable, use an infinte loop with conditional return. I.e. instead of:

while not valid_input:
    ...
    if(user_input in acceptable_input_array):
        valid_input = True
    else:
        print('Incorrect Input')

Write:

while True:
    ...
    if user_input in acceptable_input_array:
        return user_input

    print('Incorrect Input')

Know your functions

The built-in input() takes an optional prompt as string. So instead of

print("Please enter choice of", 
    acceptable_input_array,
    "for",
    prompt,
    ", blank to cancel.")

user_input = input()

You can write

user_input = input(
    "Please enter choice of "
    f"{acceptable_input_array} for "
    f"{prompt}, blank to cancel."
)

Naming

The parameter input_array of get_user_input() is not an array. In fact, there are no arrays at all in plain python. Since the parameter is a range, it should be called input_range.

Outsource common operations into functions

A common task is to filter the indices of free cells on the board. Write a function for that:

def get_free_indices(board):
    """Yields indices of free fields on the board."""

    for index, field in enumerate(board):
        if field == 0:    # or: if field is CellValue.EMPTY:
            yield index

main() function and the __main__ guard

Consider putting your running code into a main() function, that you call under the infamous if __name__ == '__main__': guard:

def main():
    """Play the game."""

    BOARD_SIZE = int(get_user_input([3,5,7],'board size'))
    board = [0] * BOARD_SIZE**2
    print_board(board)
    print("You are playing as 'X'")

    while True:
        player_turn(board)
        cpu_turn(board)


if __name__ == '__main__':
    main()

Comments

Comment what needs clarification. Don't comment what the code does. Anybody can see this by reading the code itself. Comment why your code does things, if and only if it is not obvious. All of your comments can be removed.

Docstrings

Don't document your functions with comments before them. Use docstrings instead. This:

#this function accepts a user input if it's contained in a list
def get_user_input(input_array,prompt):
    valid_input = False
    ...

should be:

def get_user_input(input_range, prompt):
    """Read user input and validate it for free fields."""

    valid_input = False
    ...
\$\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.