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)
1 Answer 1
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
...