Python Tic-Tac-Toe game

Question

I have it running well and would just like any suggestions.

_{Note: Yes I am using global variables and would like to keep it that way, at least for now. The reason is, that I am using The Great Courses to learn computer-science concepts, and the instructor insisted on using Global Variables...since we haven't learned about objects and such (got all the way to functions and that's it)}

#Objective: write a tic-tac-toe program


#Main variables
board = [[0, 0, 0], [0, 0, 0], [0, 0, 0]]
goal_score = int(input("What score would you like to play up to?"))
current_score = 0
turn = 0
round = 1
round_gone_up = bool



def InitializeGrid(board):

    for i in range(3):
        for j in range(3):
            board[i][j] = "."
    return board




def InitializeBoard():
    #initialize game
    #Initialize grid
    InitializeGrid(board)



    global turn

    # Prints out user-turn
    turn = 1
    print("Turn: ", turn)
    global round_gone_up
    round_gone_up = False




def ContinueGame(current_score, goal_score):
    #Return false if game should end, true if game not over
    if (current_score >= goal_score):
        return False
    else:
        return True



def IsValid(move):


    #move length
    if len(move) != 2:
        return False


    #move number
    if (int(move[0]) < 1):
        print("Start at index 1")
        return False


    #move must be character 'x' or 'o'
    if (move[1] != "x") and (move[1] != "o"):
        return False

    #no probs, so return true
    return True




def GetMove():
    move = input("Enter move: ")


    while not IsValid(move):
        move = input("Invalid move! Enter a valid move: ")

    return move


def PositionBoard():

    position = [[0, 0, 0], [0, 0, 0], [0, 0, 0]]

    #Indexes a board one-to-nine
    for i in range(1, 10):
        for j in range(3):
            for k in range(3):
                position[j][k] = i
                i += 1
        break
    return position


def CheckIfMoveAlreadyMade(board, move):
    space_already_taken = []
    board2 = PositionBoard()
    for i in range(3):
        for j in range(3):

            #If board-space is not blank
            if (board[i][j] != "."):
                #Store board-space in list, convert board-space to string
                space_already_taken += str(board2[i][j])

                #While move[0] in the list of board-space
                while move[0] in space_already_taken:
                    #Get new input for move
                    move = input("Space already taken! Enter a valid move: ")
                    continue
    return move



def UpdateBoard(board, move):
    board1 = PositionBoard()
    for i in range(3):
        for j in range(3):
            if (move[0] == str(board1[i][j])):
                board[i][j] = move[1]

    return board




def CheckThree(board):
    #Check each row for three-in-a-row
    for i in range(2, -1, -1):
        for j in range(2, -1, -1):
            if (board[i][j] != "."):
                if (board[i][j] == board[i][j-1]) and (board[i][j] == board[i][j-2]):

                    return board[i][j]

    #Check each column
    for j in range(3):
        for i in range(3):
            if (board[i][j] != "."):
                if (board[i][j] == board[i-1][j]) and (board[i][j] == board[i-2][j]):
                    return board[i][j]

    #Check diagonally
    if (board[1][1] != "."):
        if (((board[1][1] == board[0][0]) and (board[0][0] == board[2][2])
             or (board[1][1] == board[0][2]) and (board[0][2] == board[2][0]))):

                return board[i][j]



def CheckScore():
    global round_gone_up
    if (CheckThree(board)):
        global current_score
        global round

        #Increments score
        current_score += 1

        #Increments round
        round += 1
        #Returns boolean round_gone_up
        round_gone_up = True


        #Prints the winner
        print(CheckThree(board), " wins:", " round ", round)
    return round_gone_up



def DrawBoard(board):
    print("\n\n\n")
    print(" ---------------------------------")

    for i in range(3):
        #Draw each row
        linetodraw = ""
        for j in range(3):
            linetodraw += " | " + board[i][j]
        linetodraw += " |"
        print(linetodraw)
    print(" ---------------------------------")


def DoRound():
    DrawBoard(board)
    move = CheckIfMoveAlreadyMade(board, GetMove())

    UpdateBoard(board, CheckIfMoveAlreadyMade(board, move))
    CheckThree(board)
    CheckScore()






    global current_score
    print("Current score:", current_score)
    print("Round:", round,)





#Initialize game
InitializeBoard()

while ContinueGame(current_score, goal_score):
    # Checks if round gone up
    if (round_gone_up):
        InitializeBoard()
    DoRound()
    turn += 1
    print("Turn: ", turn)

if not ContinueGame(current_score, goal_score):
    print(CheckThree(board), "wins!")

Answer 1

Ok, here it goes. I would almost completely refactor the code to get rid of the global variables and instead pass them as function parameters where necessary. This is possible with almost all variables, leaving only a few variables in the (newly created) main loop.

---

First off, python has an official style-guide, PEP8, which programmers are encouraged to follow. It recommends using lower_case for variable and function names.

Furthermore it recommends leaving exactly two blank lines before function definitions. In addition I would get rid of many of the blank lines within you functions as well to improve readability.

---

For the board/grid there is an easier way to initialize it by using a list comprehension and the python feature that ["."]*3 == [".", ".", "."]:

Getting rid of dependency of global variable, which was not really needed anyways, since it will be completely overridden. Returning the new board is sufficient.

def initialize_board():
    return [["."]*3 for i in range(3)]

The InitializeBoard function does not really serve any purpose, except calling the board initialization itself and setting some global variables. Therefore removed.

the ContinueGame function can be simplified to:

def ContinueGame(current_score, goal_score):
    return current_score < goal_score

Your comment did not explain anything your function name did not already do. Additionally, don't put parenthesis around the conditions of an if statement, they are normal expressions and do not need to be encapsulated.

In my final code I changed this function further, to be able to keep track of two scores, one for o and one for x.

I would change the functions GetMove, IsValid and CheckIfMoveAlreadyMade. Here you should adhere to the single responsibility principle. Getting a new move from the user is currently split into two functions, which violates this principle. I would completely get rid of the function PositionBoard, because its functionality can be achieved by clever use of modulo and floor division in the other functions.

def valid(move):
    if len(move) != 2:
        return False
    # move number
    try:
        index = int(move[0])
    except ValueError:
        return False
    if index == 0:
        print("Indices are 1..9")
        return False
    # move must be character 'x' or 'o'
    if move[1] not in ["x", "o"]:
        print("Mark must be 'x' or 'o'")
        return False
    return True

Got rid of some white space here. Added a check that the number entered is actually a number. Used in instead of two different checks with and. The in range check for the index is not really needed, because a single digit can only ever be positive and in range(10), so we only need to check for 0.

def already_taken(board, move):
    index = int(move[0]) - 1
    return board[index//3][index%3] != "."

index//3 will return the row of the board (the -1 for the index is needed because the program starts counting at 1, but python at 0). The modulo operation index%3 will return the position within the row of the board. No safeguard around the call to int because it is already validated in valid.

def get_move(board):
    move = input("Enter move: ")
    while not valid(move) or already_taken(board, move):
        move = input("Invalid move! Enter a valid move: ")
    return int(move[0]), move[1]

Added the check for already_taken as another validation of move to have all prompting for a move centralized here. Returning the two separate parts of the move to get rid of some converting to strings/ints later.

The same logic as in already_taken can also be used to update board. This way it becomes an almost trivial operation and I moved it inline to where it used to be called. This gets rid of the function UpdateBoard.

The CheckThree function has a few changes as well:

The first two checks can be simplified with the use of all. Also, note here that in python it is usually frowned upon iterating over the indices of a list, just to use list[i] within the loop. Rather try iterating over the elements of the list directly.

The column check uses a nice python idiom to get a transposed nested list, zip(*board). Try it out in an interactive session to see that it works.

Python can do multiple comparisons, so a < b < c and x == y == z are all valid python.

I added a return False, in case no winner is found, enabling the use of this function to determine whether a round has ended.

def check_three(board):
    # Check each row for three-in-a-row
    for row in board:
        if row[0] != "." and all(el == row[0] for el in row):
            return row[0]
    # Check each column
    for col in zip(*board):
        if col[0] != "." and all(el == col[0] for el in col):
            return col[0]
    # Check diagonally
    if board[1][1] != ".":
        if (board[1][1] == board[0][0] == board[2][2]) or (board[1][1] == board[0][2] == board[2][0]):
                return board[1][1]
    return False

The function DrawBoard can be either left as is or shortened somewhat if you are willing to change the way the board looks slightly:

def draw(board):
    print("\n\n")
    for i, row in enumerate(board):
        print(" | ".join(row))
        if i != 2:
            print("---------")

Note that the use of join will only add the delimiters between values (so no lines on the outside). However, this is how a TicTacToe board is normally drawn, anyways:

. | . | .
---------
. | . | .
---------
. | . | .

I then added a function do_turn which takes care of a single turn by drawing the board, getting a user move and updating the board.

The whole logic of managing a round is now encapsulated in round_winner which returns the winner of the round (well, duh).

Additionally I added a main function which takes all the remaining 'global' variables (which do not really need to be global anymore since we now get the winner of the round out of the last function) and then update the score accordingly. Here I elevated current_score to a dictionary with the two keys x and o to separately track their score.

The prompting for the limit of the score is now in the function get_goal_score, where the user is prompted until he actually enters a number.

The execution of the main loop is guarded by a if __name__ = "__main__": clause. This guarantees that it is run when calling the script via python3 tictactoe.py but not when doing, in another script, from tictactoe import *.

---

What both your code and mine lack is a check against cheating. Right now, it is possible to enter these moves in a row: 1x, 2x, 3x, giving o no chance at winning at all.

Later, if you have learned about classes, you might want to refactor it again, elevating the Board to a class, and making a class for players and the whole game. This would also allow adding a CPU opponent (which would be a special case of a player).

---

Resulting code:

#Objective: write a tic-tac-toe program
def get_goal_score():
    while True:
        try:
            return int(input("What score would you like to play up to? "))
        except ValueError:
            continue


def initialize_board():
    return [["."]*3 for i in range(3)]


def continue_game(current_score, goal_score):
    return all(score < goal_score for score in current_score.values())


def valid(move):
    if len(move) != 2:
        return False
    # move number
    try:
        index = int(move[0])
    except ValueError:
        return False
    if index == 0:
        print("Indices are 1..9")
        return False
    # move must be character 'x' or 'o'
    if move[1] not in ["x", "o"]:
        print("Mark must be 'x' or 'o'")
        return False
    return True


def already_taken(board, move):
    index = int(move[0]) - 1
    return board[index//3][index%3] != "."


def get_move(board):
    move = input("Enter move: ")
    while not valid(move) or already_taken(board, move):
        move = input("Invalid move! Enter a valid move: ")
    return int(move[0]), move[1]


def check_three(board):
    # Check each row for three-in-a-row
    for row in board:
        if row[0] != "." and all(el == row[0] for el in row):
            return row[0]
    # Check each column
    for col in zip(*board):
        if col[0] != "." and all(el == col[0] for el in col):
            return col[0]
    # Check diagonally
    if board[1][1] != ".":
        if (board[1][1] == board[0][0] == board[2][2]) or (board[1][1] == board[0][2] == board[2][0]):
                return board[1][1]
    return False


def draw(board):
    print("\n\n")
    for i, row in enumerate(board):
        print(" | ".join(row))
        if i != 2:
            print("---------")


def do_turn(board):
    draw(board)
    index, mark = get_move(board)
    board[(index-1)//3][(index-1)%3] = mark


def round_winner():
    board = initialize_board()
    turn = 0
    while not check_three(board):
        print("Turn: {}".format(turn))
        do_turn(board)
        turn += 1
    draw(board)
    return check_three(board)


def main():
    goal_score = get_goal_score()
    current_score = {"x": 0, "o": 0}
    round = 1
    while continue_game(current_score, goal_score):
        print("Round: {}".format(round))
        current_score[round_winner()] += 1
        print("Current score: x: {x}, o: {o}".format(**current_score))
        round += 1
    winner = "."
    for key, val in current_score.items():
        if val == goal_score:
            winner = key
    print("Player {} wins!".format(winner))

if __name__ == "__main__":
    main()