8
\$\begingroup\$

I just got all the bugs fixed and my code is not the best. So, how can I restructure my tic-tac-toe code so that it does not take as many resources while being readable?

#!/usr/bin/env python3

#Create the grid.
def drawGrid(grid):
    #Show the grid to the user.
    for rows in grid:
        print("+---+---+---+")
        #Print each item in the row.
        for row in rows:
            print(row, end='')
        #Formatting
        print()
    print("+---+---+---+")

#Edit the Tic Tac Toe grid.
def editGrid(grid, selectedRow, selectedColumn, player):
    row = grid[selectedRow - 1]
    #Determine which player is editing the
    #grid.      
    if player == " X ":
        #Determine what to do based on the column selected.
        if selectedColumn == 1:
            #Check to see if a player is in the spot selected.
            if row[selectedColumn] == " O " or row[selectedColumn] == " X ":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player
        elif selectedColumn == 2:
            if row[selectedColumn + 1] == " O " or row[selectedColumn + 1] == " X ":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn + 1] = player
        elif selectedColumn == 3:
            if row[selectedColumn + 2] == " O " or row[selectedColumn + 2] == " X ":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn + 2] = player
    else:
        #Determine what to do based on the column selected.
        if selectedColumn == 1: 
            #Check to see if a player is in the spot selected.
            if row[selectedColumn] == " O " or row[selectedColumn] == " X ":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player
        elif selectedColumn == 2:
            if row[selectedColumn + 1] == " O " or row[selectedColumn + 1] == " X ":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn + 1] = player
        elif selectedColumn == 3:
            if row[selectedColumn + 2] == " O " or row[selectedColumn + 2] == " X ":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn + 2] = player

#Determine if there are any winners.
def checkWinner(grid, player):
    if (grid[0][1] == player and grid[0][3] == player) and grid[0][5] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[1][1] == player and grid[1][3] == player) and grid[1][5] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[2][1] == player and grid[2][3] == player) and grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[0][1] == player and grid[1][3] == player) and grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[0][3] == player and grid[1][3] == player) and grid[2][3] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[0][5] == player and grid[1][5] == player) and grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[0][1] == player and grid[1][3] == player) and grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[0][5] == player and grid[1][3] == player) and grid[2][1] == player:
        print("\n" + player + "wins!")
        return True
    elif (grid[0][1] == player and grid[1][1] == player) and grid[2][1] == player:
        print("\n" + player + "wins!")
        return True
    else:
        return False

#Ask the user which column to occupy.
def getRow():
    while True:
        #Catch any invalid data.
        try:
            row = int(input("Pick a row (1, 2, 3): "))
            #Check the validity of the input
            #data.
            while row < 1 or row > 3:
                print("Please enter an integer bewteen 1 and 3. Try again.")
                row = int(input("Pick a row (1, 2, 3): "))
            return row
        except ValueError:
            print("Please input an integer between 1 and 3. Try again.")

#Ask the user which column to occupy.
def getColumn():
    while True:
        #Catch any invalid data.
        try:
            column = int(input("Pick a row (1, 2, 3): "))
            #Check the validity of the input
            #data.
            while column < 1 or column > 3:
                print("Please enter an integer bewteen 1 and 3. Try again.")
                column = int(input("Pick a row (1, 2, 3): "))
            return column
        except ValueError:
            print("Please input an integer between 1 and 3. Try again.")

def main():
    print("Welcome to Tic Tac Toe\n")
    #Assume the user wants to play.
    userChoice = "y"
    while userChoice.lower() == "y":
        #Form the grid
        grid = [["|","   ","|", "   ","|", "   ","|"],
                ["|","   ","|", "   ","|", "   ","|"],
                ["|","   ","|", "   ","|", "   ","|"]]
        drawGrid(grid)
        #Set the default number of turns.
        numberOfTurns = 1
        isWon= False
        isTie = False
        #Run until someone wins or it ties.
        while isWon == False and isTie == False:
            #Catch any errors with the input.
            if numberOfTurns % 2 != 0:
                print("\nX's Turn")
                #Set who goes this turn and what
                #marker to use.
                player = " X "
                #Get input.
                row = getRow()
                column = getColumn()
            else:
                print("\nO's Turn")
                #Set who goes this turn and what
                #marker to use.
                player = " O "
                #Get input.
                row = getRow()
                column = getColumn()
            print()
            #Edit the gird with the values given.
            editGrid(grid, row, column, player)
            #Draw the edited grid.
            drawGrid(grid)
            #Determine if the programs needs to terminate.
            isWon = checkWinner(grid, player)
            #Check the status of the game.
            if isWon == False and isTie == False:
                #Keep track of how many spaces
                #are taken up.
                tokenCounter = 0
                for rows in grid:
                    for space in rows:
                        #Determine if all there spaces
                        #are taken up.
                        if space == " X " or space == " O ":
                            tokenCounter += 1
                #Determine if there was a tie.
                if tokenCounter == 9 and isWon == False:
                    isTie = True
                    print("\nIt is a tie!\n")
            #Next players goes.
            numberOfTurns += 1
        print("Round over!\n")
        userChoice = input("Do you want to play another round? (y / n) ")
        while userChoice.lower() != "y" and userChoice.lower() != "n":
            print("Please type \"y\"  or \"n\". Try again.")
            userChoice = input("Do you want to play another round? (y / n) ")
        print()
    print("Bye!")

if __name__ == "__main__":
    main()
\$\endgroup\$
5
\$\begingroup\$

Your overall breakdown looks good. There are a few problems in the organization and logic:

Create a newGrid function

In main you have this code:

    #Form the grid
    grid = [["|","   ","|", "   ","|", "   ","|"],
            ["|","   ","|", "   ","|", "   ","|"],
            ["|","   ","|", "   ","|", "   ","|"]]
    drawGrid(grid)

The setting of the grid array leaks implementation details out to main, which should not know them. Instead, I suggest encapsulating the grid construction in a function, newGrid (or whatever you like) that just returns the appropriate value. This will let you change how the grid is implemented (see below) without having to know about it in main.

Change the grid implementation

Your grid stores both separator characters and cell contents. Worse, it also stores white space with the cell contents. It would be better to handle the formatting of the grid in the actual drawGrid function (since that is its job!) and leave the grid data structure to just store data.

Get rid of isWon and isTie

You maintain two variables that supposedly describe the termination status of the program. You do a bad job of checking them:

        if isWon == False and isTie == False:

For boolean variables, please evaluate the value directly rather than comparing it with True or False, like so:

        if not isWon and not IsTie:

Moreover, none of these variables are necessary. You have a function that checks if the game is over. Simply use that function directly in your while loop:

while not checkWinner(...):

Of course, haveWinner might be a better name than checkWinner.

Break down your main function

Currently your main function consists of a loop to play multiple games in series, containing a loop to play each alternating turn, plus logic to display the end-of-game results.

Break down your main into a separate function to play one game, plus a loop to play multiple games by calling that function:

def main():
    while play_another_game:
        play_game()

Merge getRow and getColumn functions

You have two functions that obviously are copies, since the getColumn function still asks the user to "Pick a row".

I'd suggest merging them into a single function that just prints a different word, which you can pass as a parameter:

def getIndex(word):
    print("Pick a {}".format(word))
    ...

You can then define getRow and getColumn as calling the common underlying function and passing a different string:

def getRow():
    return getIndex("row")

Invert logic in checkWinner

Your checkWinner function contains various tests that look for a winner. Under each check, you try to print a message and return, resulting in 8 different copies of the printed message.

I suggest you first look for a way to compress the test-expressions using list of tuples (or tuples of tuples) to store the various winning combinations as data, rather than code. Try the all built-in function for this:

ways_to_win = (
    ((0,0), (0,1), (0,2)),
    ((1,0), (1,1), (1,2)),
    ((2,0), (2,1), (2,2)),
    ...
 )

for way2win in ways_to_win:
    if all(grid[x][y] == player for x,y in way2win):
        break
else:
    return False

print("Winner!")
return True
\$\endgroup\$
4
\$\begingroup\$

The answers so far do not address the big design issues properly

Separate I/O from the internal data model

You have a horrible mix between internal data representation and concrete I/O formatting. grid contains extra grid characters for output, on the other hand there is drawGrid handling the extra grid lines. This would not make it difficult to do a different output (graphic?) but also causes enormous pain in column indexing. The data representaion shall contain only cell values in a 3x3 matrix (you may stick to character representation). Drawing separators shall be pusehd to drawGrid.

grid = [[" "," "," "],
        [" "," "," "],
        [" "," "," "]]

Before adapting drawGrid we have a look at the names which are not reflecting reality. the loop for rows in grid: shall be for row in grid: because it is a single row. likewise for row in rows: should be for part in row: as there are several data/spacing/separation parts in there. That said we change drawGrid to (still not perfect)

def drawGrid(grid):
    #Show the grid to the user.
    for row in grid:
        print("+---+---+---+")
        print("| " + " | ".join(row) + " |")
    print("+---+---+---+")

We also have to remove all the column indexing offset stuff as grid is now a real representation of our data. Also we have to remove the spacing from the data values like " X " used to test/set values.


Note: even in your representation it should not have been

grid = [["|","   ","|", "   ","|", "   ","|"],

but

grid = [["| "," "," | ", " "," | ", " "," |"],

Note: whenever you have string literals used more than once define a variable and use that one in all tests and assignments. in case of a typo you are more likely to get an interpreter NameError while misspelled literals may cause really ugly bugs. so e. g. do

player1 = " X "
# if player == " X ":
#     #Determine what to do based on the column selected.
#     if selectedColumn == 1:
#         #Check to see if a player is in the spot selected.
#         if row[selectedColumn] == " O " or row[selectedColumn] == " X ":
if player == player1:
    #Determine what to do based on the column selected.
    if selectedColumn == 1:
        #Check to see if a player is in the spot selected.
        if row[selectedColumn] == player2 or row[selectedColumn] == player1:

throughout your code


after removing spacing (17 string constants) and column index offsets (12 times) we also hav to fix column indexing to be 0-based. If you have to present indexing to the user 1-based then do so. but always keep all internal indices 0-based so you can use them directly as list indices. We convert the user input immediately

def getRow():
    # ...
    return row - 1

def getColumn():
    # ...
    return column - 1

we also fix all the column indices in checkWinner (where we find 9 winning cases which is one to many).

Do not repeat yourself

now we have a close look at editGrid as it looks now.

def editGrid(grid, selectedRow, selectedColumn, player):
    row = grid[selectedRow]
    #Determine which player is editing the
    #grid.      
    if player == "X":
        #Determine what to do based on the column selected.
        if selectedColumn == 0:
            #Check to see if a player is in the spot selected.
            if row[selectedColumn] == "O" or row[selectedColumn] == "X":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player
        elif selectedColumn == 1:
            if row[selectedColumn] == "O" or row[selectedColumn] == "X":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player
        elif selectedColumn == 2:
            if row[selectedColumn] == "O" or row[selectedColumn] == "X":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player
    else:
        #Determine what to do based on the column selected.
        if selectedColumn == 0: 
            #Check to see if a player is in the spot selected.
            if row[selectedColumn] == "O" or row[selectedColumn] == "X":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player
        elif selectedColumn == 1:
            if row[selectedColumn] == "O" or row[selectedColumn] == "X":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player
        elif selectedColumn == 2:
            if row[selectedColumn] == "O" or row[selectedColumn] == "X":
                print("\nA player is already there.\n")
            else:
                row[selectedColumn] = player

there is not only to completely identical big clauses in the if-else but also inside the different cases are identical. the whole function collapses to

def editGrid(grid, selectedRow, selectedColumn, player):
    row = grid[selectedRow]
    if row[selectedColumn] == "O" or row[selectedColumn] == "X":
        print("\nA player is already there.\n")
    else:
        row[selectedColumn] = player

while the big clauses were identical from the start the smaller cases did differ only because of the bad data representation.

Avoid error prone repetition of constants

now let's have a closer look at checkWinner. there is one diagonal checked twice, we delete that one. the cause for this error is editing by copy paste and altering values. with a little python list slicing and comprehension we can make this code shorter and thus less error prone

def checkWinner(grid, player):
    flat = [cell for row in grid for cell in row]
    lines = grid + [flat[i::3] for i in (0,1,2)] + [flat[0::4]] + [flat[2::2][:3]]
    win = any(all(cell==player for cell in line) for line in lines)
    if win:
        print("\n" + player + "wins!")
    return win

Again - do not repeat yourself

okay, let's go further down the road. we find two functions getRow and getColumn where one is an obvious copy of the other. getColumn even prompts for a row "Pick a row (1, 2, 3): ". you shall not repeat yourself . While other cases of repeated code were cased by an initially bad choice regarding internal representation, this is a case without an excuse. as the functions are implemented one could safely call getRow for getting a column and vice versa. So we delete getColumn without replacement. If we wan to prompt for "Row" or "Column" we make that a parameter.

some smaller repeated code

we continue into main where we replaced the calls to getColumn

if numberOfTurns % 2 != 0:
    print("\nX's Turn")
    #Set who goes this turn and what
    #marker to use.
    player = "X"
    #Get input.
    row = getRow()
    column = getRow()
else:
    print("\nO's Turn")
    #Set who goes this turn and what
    #marker to use.
    player = "O"
    #Get input.
    row = getRow()
    column = getRow()

there we again see a lot of repeated code the only thing differing is the player. we refactor and pull some code outside the clause

if numberOfTurns % 2 != 0:
    #Set who goes this turn and what
    #marker to use.
    player = "X"
else:
    player = "O"
print("\n" + player + "'s Turn")
#Get input.
row = getRow()
column = getRow()

still there is an if clause which we could make shorter. there are numerous techniques to toggle/select/cycle values from a limited set. here i am tempted to use a very lazy one (as the number of iterations is small)

player = "_XOXOXOXOX"[numberOfTurns]
print("\n" + player + "'s Turn")
#Get input.
row = getRow()
column = getRow()

unfortunately we find a big bug - if editGrid fails we switch user (and run out of player indices). we fix that later.

we continue with

if isWon == False and isTie == False:
    #Keep track of how many spaces
    #are taken up.
    tokenCounter = 0
    for rows in grid:
        for space in rows:
            #Determine if all there spaces
            #are taken up.
            if space == "X" or space == "O":
                tokenCounter += 1
    #Determine if there was a tie.
    if tokenCounter == 9 and isWon == False:
        isTie = True
        print("\nIt is a tie!\n")

first isTie will always be false here. A tied game would not have continued and the current tie status is still to be checked. Also there is something like a token counter already up to date (numberOfTurns). Again there is a superflous check isWon == False when the counter reaches 9. This could be reduced to

if not isWon and numberOfTurns == 9:
    isTie = True
    print("\nIt is a tie!\n")

However there is good reason to have detecting a tie not depend on some variable intended for different use. it shall be a function (so you could e. g. detect a tie also before the grid is full).

def isTied(grid):
    return all(" " not in row for row in grid)

we use like

if not isWon and isTied(grid):
    isTie = True
    print("\nIt is a tie!\n")

Proper error handling

so now the big bug. if we cannot place the player in editGrid we switch user. if something is an error report it to the caller. that does not mean to print something to the screen but to return an error condition or raise an error. Output to console shall be done from main only. so we refactor editGrid to return success and remove the print

def editGrid(grid, selectedRow, selectedColumn, player):
    row = grid[selectedRow]
    if row[selectedColumn] == " ":
        row[selectedColumn] = player
        return True
    return False

while in main we check the success, print the error and repeat the input.

player = "_XOXOXOXOX"[numberOfTurns]
while True:
    print("\n" + player + "'s Turn")
    #Get input.
    row = getRow()
    column = getRow()
    print()
    #Edit the gird with the values given.
    if editGrid(grid, row, column, player):
        break
    print("\nA player is already there.\n")
#Draw the edited grid.
drawGrid(grid)

No I/O from core functions

We already did remove a print from editGrid. We now do that for others as well. When you do print from core functions you either

  • mix user I/O with core
  • do error/warning messages

the first definitely do not belong there, this shall be done in main or in a UI. for the latter if no user interaction is required this could be logging. if requred it should be done on the dedicated UI. getting the message there could be via return values or more pythonic via raised exceptions.

so we remove the print from checkWinner

def checkWinner(grid, player):
    flat = [cell for row in grid for cell in row]
    lines = grid + [flat[i::3] for i in (0,1,2)] + [flat[0::4]] + [flat[2::2][:3]]
    win = any(all(cell==player for cell in line) for line in lines)
    return win

and move it to the UI (main here)

isWon = checkWinner(grid, player)
#Check the status of the game.
if isWon:
    print("\n" + player + "wins!")

finally we end up with

#!/usr/bin/env python3

def drawGrid(grid):
    #Show the grid to the user.
    for row in grid:
        print("+---+---+---+")
        print("| " + " | ".join(row) + " |")
    print("+---+---+---+")

def editGrid(grid, selectedRow, selectedColumn, player):
    row = grid[selectedRow]
    if row[selectedColumn] == " ":
        row[selectedColumn] = player
        return True
    return False

def checkWinner(grid, player):
    flat = [cell for row in grid for cell in row]
    lines = grid + [flat[i::3] for i in (0,1,2)] + [flat[0::4]] + [flat[2::2][:3]]
    win = any(all(cell==player for cell in line) for line in lines)
    return win

def isTied(grid):
    return all(" " not in row for row in grid)

#Ask the user which column to occupy.
def getRow():
    while True:
        #Catch any invalid data.
        try:
            row = int(input("Pick a row (1, 2, 3): "))
            #Check the validity of the input
            #data.
            while row < 1 or row > 3:
                print("Please enter an integer bewteen 1 and 3. Try again.")
                row = int(input("Pick a row (1, 2, 3): "))
            return row-1
        except ValueError:
            print("Please input an integer between 1 and 3. Try again.")

def main():
    print("Welcome to Tic Tac Toe\n")
    #Assume the user wants to play.
    userChoice = "y"
    while userChoice.lower() == "y":
        #Form the grid
        grid = [[" "," "," "],
                [" "," "," "],
                [" "," "," "]]
        drawGrid(grid)
        #Set the default number of turns.
        numberOfTurns = 1
        isWon= False
        isTie = False
        #Run until someone wins or it ties.
        while isWon == False and isTie == False:
            #Catch any errors with the input.
            player = "_XOXOXOXOX"[numberOfTurns]
            while True:
                print("\n" + player + "'s Turn")
                #Get input.
                row = getRow()
                column = getRow()
                print()
                #Edit the gird with the values given.
                if editGrid(grid, row, column, player):
                    break
                print("\nA player is already there.\n")
            #Draw the edited grid.
            drawGrid(grid)
            #Determine if the programs needs to terminate.
            isWon = checkWinner(grid, player)
            #Check the status of the game.
            if isWon:
                print("\n" + player + "wins!")
            if not isWon and isTied(grid):
                isTie = True
                print("\nIt is a tie!\n")

            #Next players goes.
            numberOfTurns += 1
        print("Round over!\n")
        userChoice = input("Do you want to play another round? (y / n) ")
        while userChoice.lower() != "y" and userChoice.lower() != "n":
            print("Please type \"y\"  or \"n\". Try again.")
            userChoice = input("Do you want to play another round? (y / n) ")
        print()
    print("Bye!")

if __name__ == "__main__":
    main()

we have

  • eliminated a bad data model
  • removed duplicate code
  • fixed a serious bug (and a smaller one)
  • got a separation between user interface and core functionality

Todos left:

  • write some test cases
  • extract game logic from main
    • still to complex
    • make it testable
  • improve some code (now locally)
\$\endgroup\$
3
\$\begingroup\$

editGrid()

version 1:

In editGrid() whether the player is worth X or O, you are taking the same actions, and you find yourself with duplicate code. The truth is that you do not even need to test the value of the player variable inside this function.

By accepting this premise and respecting the DRY principle you are already able to get rid of half of its code:

def editGrid(grid, selectedRow, selectedColumn, player):
    row = grid[selectedRow - 1]
    #Determine what to do based on the column selected.
    if selectedColumn == 1:
        #Check to see if a player is in the spot selected.
        if row[selectedColumn] == " O " or row[selectedColumn] == " X ":
            print("\nA player is already there.\n")
        else:
            row[selectedColumn] = player
    elif selectedColumn == 2:
        if row[selectedColumn + 1] == " O " or row[selectedColumn + 1] == " X ":
            print("\nA player is already there.\n")
        else:
            row[selectedColumn + 1] = player
    elif selectedColumn == 3:
        if row[selectedColumn + 2] == " O " or row[selectedColumn + 2] == " X ":
            print("\nA player is already there.\n")
        else:
            row[selectedColumn + 2] = player

version 2:

Now things get clear and we can refactor the above code even deeper and cleaner: You can easily notice that row[selectedColumn + something] is calculated based on selectedColumn - 1 for the three cases (possible values) of selectedColumn. This means you can easily refactor version 1 above to this shorter version:

def editGrid(grid, selectedRow, selectedColumn, player):
    row = grid[selectedRow - 1]
    if row[2 * selectedColumn - 1] in [" X ", " 0 "]:
        print("\nA player is already there.\n")
    else:
        row[2 * selectedColumn - 1] = player

getColumn()

Minor error

There is a minor error in the message you display to the user: the user has to type a column value, not a row.

Improvement

The try block contains too much code compared to the rest of the function. This is in contradiction to the good practice where we should minimize the try block at maximum, deal with the exception in except, and then leave the rest of trash to the else statement if needed.

This function suffers also from duplicate code which you can easily get rid of if you respect the previous advise:

def getColumn():
    while True:
        try:
            column = int(input("Pick a column, (1, 2, 3): "))           
        except ValueError:
            print("You typed a non integer value. Try again.")
        else:
           if  1 <= column <= 3:
               return column
           else:
               print("The integer value must be between 1 and 3. Try again.")

Note how I customized the error messages a little better.

Now, what if the user aims to exit your application in the middle of getColumn() execution? The Ctrl + C natural reflex spits out an exception instead of a smooth Good bye!. Let us deal with it:

def getColumn():
    while True:
        try:
            column = int(input("Pick a column, (1, 2, 3): "))           
        except ValueError:
            print("You typed a non integer value. Try again.")
        except KeyboardInterrupt:
            print("\nThanks... good bye!")
            quit(0)
        else:
           if  1 <= column <= 3:
               return column
           else:
               print("The integer value must be between 1 and 3. Try again.")

getRow()

With the same steps as the ones for getColumn() you can land on this version:

def getRow():
    while True:
        try:
            row = int(input("Pick a row, (1, 2, 3): "))           
        except ValueError:
            print("You typed a non integer value. Try again.")
        except KeyboardInterrupt:
            print("\nThanks... good bye!")
            quit(0)
        else:
           if  1 <= row <= 3:
               return row
           else:
               print("The integer value must be between 1 and 3. Try again.")

Refactoring getColumn() and getRow()

You can easily notice that the only difference between getRow() and getColumn() are the functions' names. I mean you duplicate the code and violate the DRY principle.

One way to fix this issue is to get both inputs from a unique function we can call get_row_and_column(). This function returns a tuple where the first element corresponds to the row, and the second one to the column typed by the user:

def get_row_and_column():
    while True:
        try:
            row = int(input('row (between 1 and 3): '))
            column = int(input('column (between 1 and 3): '))
        except ValueError:
            print("You typed a non integer value. Try again.")
        except KeyboardInterrupt:
            print("\nThanks... good bye!")
            quit(0)
        else:  
           res = all(1 <= i <= 3 for i in (row, column)) 
           if res:
               return row, column
           else:
               print("The integer value must be between 1 and 3. Try again.")

Now in your main() function, replace getRow() calls by get_row_and_column()[0], and getColumn() calls by get_row_and_column()[1]

checkWinner()

The previous answer gave you already a good improvement. My aim is not to provide better than that but just to help you with some things so that you will not re-do them in the future, hopefully.

The first observation is that the lines of this function's body are unnecessarily long. You can replace those lines by shorter ones:

def checkWinner(grid, player):
    if grid[0][1] == grid[0][3] == grid[0][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[1][1] ==  grid[1][3] == grid[1][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[2][1] == grid[2][3] ==  grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][1] ==  grid[1][3] == grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][3] == grid[1][3] == grid[2][3] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][5] ==  grid[1][5] ==  grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][1] ==  grid[1][3] == grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][5] == grid[1][3] ==  grid[2][1] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][1] == grid[1][1] ==  grid[2][1] == player:
        print("\n" + player + "wins!")
        return True
    else:
        return False

And without going deeper, you can ever refactor the 3 first conditions:

def checkWinner(grid, player):
    for i in range(0, 3):
       if grid[i][1] == grid[i][3] == grid[i][5] == player:
           print("\n" + player + "wins!")
           return True    
    if grid[0][1] ==  grid[1][3] == grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][3] == grid[1][3] ==  grid[2][3] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][5] == grid[1][5] == grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][1] == grid[1][3] ==  grid[2][5] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][5] == grid[1][3] == grid[2][1] == player:
        print("\n" + player + "wins!")
        return True
    elif grid[0][1] == grid[1][1] ==  grid[2][1] == player:
        print("\n" + player + "wins!")
        return True
    else:
        return False

Additional notes

Last but not the least, these notes should be cared about when coding in Python:

  1. Lot of your functions and variables follow the camelCase scheme. In Python, we rather adopt the snake_case way of writing. This means functions editGrid() and drawGrid() become edit_grid() and draw_grid() respectively. The same thing goes for variables like selectedColumn and selectRaw which should be written selected_column and select_raw respectively. You may refer to the naming conventions highlighted in PEP8.
  2. IMHO, the whole of your comments should be removed because they just describe what is already obvious through the instructions.
  3. There are a little more improvements to perform on the main() function, but for now I am too lazy to do them. Maybe someone could continue through a new answer.
\$\endgroup\$
  • \$\begingroup\$ Another question: When is it "okay" to use "While True:"? I was told it was frowned upon to do that. That is why the first version (the one in my first post is the revised, bug-free version) of my code was the way it was. \$\endgroup\$ – Daniel Loughran Nov 23 '17 at 20:53
  • 1
    \$\begingroup\$ It is okay to use while True when your intent is to loop forever. You do this in getRow, but that function doesn't really want to loop forever - it wants to loop until it gets a good value. So try something like while value is None: instead. \$\endgroup\$ – Austin Hastings Nov 24 '17 at 0:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.