-4
\$\begingroup\$

Here is my code:

def printBoard(board):  # this prints the board
    print("         |         |         ")
    print(f"    {board[0][0]}    |    {board[0][1]}    |    {board[0][2]}    ")
    print("         |         |         ")
    print("---------|---------|---------")
    print("         |         |         ")
    print(f"    {board[1][0]}    |    {board[1][1]}    |    {board[1][2]}    ")
    print("         |         |         ")
    print("---------|---------|---------")
    print("         |         |         ")
    print(f"    {board[2][0]}    |    {board[2][1]}    |    {board[2][2]}    ")
    print("         |         |         ")


def isGameOver(board, playerOneSprite, playerTwoSprite): #checks if eiither player won
    for i in range(3):
        if board[i][0] == board[i][1] and board[i][1] == board[i][2]:
            if board[i][0] == playerOneSprite:
                print("Player one wins!")
                return True
            elif board[i][0] == playerTwoSprite:
                print("PlayerTwoWins!")
                return True
        if board[0][i] == board[1][i] and board[1][i] == board[1][i]:
            if board[0][i] == playerOneSprite:
                print("Player one wins!")
                return True
            elif board[0][i] == playerTwoSprite:
                print("PlayerTwoWins!")
                return True
    if board[0][0] == board[1][1] and board[1][1] == board[2][2]:
        if board[0][0] == playerOneSprite:
            print("Player one wins!")
            return True
        elif board[0][0] == playerTwoSprite:
            print("PlayerTwoWins!")
            return True
    if board[0][2] == board[1][1] and board[1][1] == board[2][0]:
        if board[0][2] == playerOneSprite:
            print("Player one wins!")
            return True
        elif board[0][2] == playerTwoSprite:
            print("PlayerTwoWins!")
            return True
    return False


def switchTurns(playerOneSprite, playerTwoSprite, isPlayerOneTurn, board): #controls the flow of the game
    printBoard(board)
    if isPlayerOneTurn:
        row = int(input("Player one, enter row: "))-1
        column = int(input("Player one, enter column: "))-1
        if (row > 3 or row < 1) or (column > 3 or column < 1):
            print("Out of range!")
            switchTurns(playerOneSprite, playerTwoSprite, isPlayerOneTurn, board)
        if board[row][column] == '-':
            board[row][column] = playerOneSprite
    else:
        row = int(input("Player two, enter row: "))-1
        column = int(input("Player two, enter column: "))-1
        if (row > 3 or row < 1) or (column > 3 or column < 1):
            print("Out of range!")
            switchTurns(playerOneSprite, playerTwoSprite, isPlayerOneTurn, board)
        if board[row][column] == '-':
            board[row][column] = playerTwoSprite
    if isGameOver(board, playerOneSprite, playerTwoSprite):
        printBoard(board)
    else:
        switchTurns(playerOneSprite, playerTwoSprite, not isPlayerOneTurn, board)


switchTurns('X', 'O', True, board = [['-', '-', '-'], ['-', '-', '-'], ['-', '-', '-']]) #driver code

Is there a way to make this code smaller, more compact and readable? Right now it is 72 lines long. I am hoping to get it below 60 lines, maybe even 50... For instance, I have noticed that in the switchTurns() function some of the code is duplicated, but I am not sure how to implement that without sacrificing some clarity in the game... Also, are there any improvements I can potentially make?

\$\endgroup\$
2
  • 3
    \$\begingroup\$ Have you played your game to test it? Did it work for you, as expected? Did it really? You ask for a row/column and subtract 1 from the input. And then you test if the values are below 1 or above 3. This means the user must enter row/column values in the range of 2 to 4 to get past validation. But then, after the subtract 1, the values are in the range 1 to 3, but your board indices must be between 0 and 2, so you should see IndexError exceptions raised. This is non-working code, and is off-topic for Code Review until you have fixed obvious issues causing the game to malfunction. \$\endgroup\$
    – AJNeufeld
    Sep 18, 2020 at 21:01
  • \$\begingroup\$ Sorry about the fact that my code doesn't work... \$\endgroup\$
    – fartgeek
    Sep 21, 2020 at 0:37

1 Answer 1

0
\$\begingroup\$

I'm assuming that you are using python 3

When I ran the code, there were some errors.

First, there was an invalid syntax in the printBoard function. Here is the fixed version:

def printBoard(board):  # this prints the board
    print("         |         |         ")
    print("    ", board[0][1], "    |    ", board[0][1], "    |    ", board[0][2], "    ", sep='')
    print("         |         |         ")
    print("---------|---------|---------")
    print("         |         |         ")
    print("    ", board[0][1], "    |    ", board[0][1], "    |    ", board[0][2], "    ", sep='')
    print("         |         |         ")
    print("---------|---------|---------")
    print("         |         |         ")
    print("    ", board[0][1], "    |    ", board[0][1], "    |    ", board[0][2], "    ", sep='')
    print("         |         |         ")

The commas will concatenate the strings together and sep='' sets up the concatenation so that there are no spaces between the concatenated strings.

Next, the logic of inputting the position on the board seems to be off.

row = int(input("Player one, enter row: "))-1
column = int(input("Player one, enter column: "))-1
if (row > 3 or row < 1) or (column > 3 or column < 1):

Since you've subtracted 1 from both the inputted row and column, the condition for the if statement should be:

if (row < 0 or row > 2) or (column < 0 or column > 2):

Also, you seem to be recursing the function again when there is an invalid input. A much cleaner way is to use while loops as such:

while True:
    row = int(input("Player one, enter row: ")) - 1
    column = int(input("Player one, enter column: ")) - 1

    if 0 <= row <= 2 and 0 <= column <= 2 and board[row][column] == '-':
        board[row][column] = playerOneSprite
        break
    else:
        if not (0 <= row <= 2 and 0 <= column <= 2):
            print("Out of range!")
        else:
            print("Box already filled")

Note that I've combined the condition of checking if the box is filled or not and checking that the value of row and column inputted are in range.

Since this is used twice for player one and player two, I've put this block of code into a function:

def inputPosition(board, player, sprite):
    while True:
        row = int(input("Player " + player + ", enter row: ")) - 1
        column = int(input("Player " + player + ", enter column: ")) - 1

        if 0 <= row <= 2 and 0 <= column <= 2 and board[row][column] == '-':
            board[row][column] = sprite
            break
        else:
            if not (0 <= row <= 2 and 0 <= column <= 2):
                print("Out of range!")
            else:
                print("Box already filled")

So now, switchTurns looks like this:

def switchTurns(playerOneSprite, playerTwoSprite, isPlayerOneTurn, board): #controls the flow of the game
    printBoard(board)

    if isPlayerOneTurn: inputPosition(board, "one", playerOneSprite)
    else: inputPosition(board, "two", playerTwoSprite)
  
    if isGameOver(board, playerOneSprite, playerTwoSprite):
        printBoard(board)
    else:
        switchTurns(playerOneSprite, playerTwoSprite, not isPlayerOneTurn, board)

Finally, the function isGameOver can be more efficient:

def isGameOver(board, playerOneSprite, playerTwoSprite): #checks if eiither player won
    for i in range(3):
        if board[i][0] == board[i][1] and board[i][1] == board[i][2]:
            if board[i][0] == playerOneSprite: return 1
            elif board[i][0] == playerTwoSprite: return 2
        if board[0][i] == board[1][i] and board[1][i] == board[2][i]:
            if board[0][i] == playerOneSprite: return 1
            elif board[0][i] == playerTwoSprite: return 2

    if board[0][0] == board[1][1] and board[1][1] == board[2][2]:
        if board[0][0] == playerOneSprite: return 1
        elif board[0][0] == playerTwoSprite: return 2

    if board[0][2] == board[1][1] and board[1][1] == board[2][0]:
        if board[0][2] == playerOneSprite: return 1
        elif board[0][2] == playerTwoSprite: return 2

    for i in range(3):
        for j in range(3):
            if board[i][j] == '-': return 0
    
    return -1

The function will return -1 if it is a draw, 0 if there are no winners, 1 if player one won and 2 if player two won.

I've also added a for loop at the end to check through all the boxes whether players can make more moves or not.

Also, there was an error with the code where you've put:

if board[0][i] == board[1][i] and board[1][i] == board[1][i]:

Now with this function, switchTurns looks like this:

def switchTurns(playerOneSprite, playerTwoSprite, isPlayerOneTurn, board): #controls the flow of the game
    printBoard(board)

    if isPlayerOneTurn: inputPosition(board, "one", playerOneSprite)
    else: inputPosition(board, "two", playerTwoSprite)
    
    gameStatus = isGameOver(board, playerOneSprite, playerTwoSprite)

    if gameStatus == 0:
        switchTurns(playerOneSprite, playerTwoSprite, not isPlayerOneTurn, board)
    else:
        printBoard(board)
        if gameStatus == 1: print("Player 1 wins")
        elif gameStatus == 2: print("Player 2 wins")
        elif gameStatus  == -1: print("It's a tie")
        return

I've added printBoard(board) so that the players can see the final state of the board.

Now here is the final code:

def printBoard(board):  # this prints the board
    print("         |         |         ")
    print("    ", board[0][0], "    |    ", board[0][1], "    |    ", board[0][2], "    ", sep='')
    print("         |         |         ")
    print("---------|---------|---------")
    print("         |         |         ")
    print("    ", board[1][0], "    |    ", board[1][1], "    |    ", board[1][2], "    ", sep='')
    print("         |         |         ")
    print("---------|---------|---------")
    print("         |         |         ")
    print("    ", board[2][0], "    |    ", board[2][1], "    |    ", board[2][2], "    ", sep='')
    print("         |         |         ")

def isGameOver(board, playerOneSprite, playerTwoSprite): #checks if eiither player won
    for i in range(3):
        if board[i][0] == board[i][1] and board[i][1] == board[i][2]:
            if board[i][0] == playerOneSprite: return 1
            elif board[i][0] == playerTwoSprite: return 2
        if board[0][i] == board[1][i] and board[1][i] == board[2][i]:
            if board[0][i] == playerOneSprite: return 1
            elif board[0][i] == playerTwoSprite: return 2

    if board[0][0] == board[1][1] and board[1][1] == board[2][2]:
        if board[0][0] == playerOneSprite: return 1
        elif board[0][0] == playerTwoSprite: return 2

    if board[0][2] == board[1][1] and board[1][1] == board[2][0]:
        if board[0][2] == playerOneSprite: return 1
        elif board[0][2] == playerTwoSprite: return 2

    for i in range(3):
        for j in range(3):
            if board[i][j] == '-': return 0
    
    return -1

def inputPosition(board, player, sprite):
    while True:
        row = int(input("Player " + player + ", enter row: ")) - 1
        column = int(input("Player " + player + ", enter column: ")) - 1

        if 0 <= row <= 2 and 0 <= column <= 2 and board[row][column] == '-':
            board[row][column] = sprite
            break
        else:
            if not (0 <= row <= 2 and 0 <= column <= 2):
                print("Out of range!")
            else:
                print("Box already filled")

def switchTurns(playerOneSprite, playerTwoSprite, isPlayerOneTurn, board): #controls the flow of the game
    printBoard(board)

    if isPlayerOneTurn: inputPosition(board, "one", playerOneSprite)
    else: inputPosition(board, "two", playerTwoSprite)
    
    gameStatus = isGameOver(board, playerOneSprite, playerTwoSprite)

    if gameStatus == 0:
        switchTurns(playerOneSprite, playerTwoSprite, not isPlayerOneTurn, board)
    else:
        printBoard(board)
        if gameStatus == 1: print("Player 1 wins")
        elif gameStatus == 2: print("Player 2 wins")
        elif gameStatus == -1: print("It's a tie")
        return

switchTurns('X', 'O', True, board = [['-', '-', '-'], ['-', '-', '-'], ['-', '-', '-']]) #driver code

This is 68 lines of code. If you remove the spaces between the lines of code, it will become 59 lines. However, you should try to avoid focusing on the number of lines of code you haves. You shouldn't be hesitant to use more lines of code if it will make your code tidier.

Also when you post answers to code review, make sure your code does not include any errors.

\$\endgroup\$
3
  • 5
    \$\begingroup\$ Try to remember that we don't review code that doesn't work. This question will be closed. \$\endgroup\$
    – pacmaninbw
    Sep 18, 2020 at 23:24
  • \$\begingroup\$ Python 3.6 and later supports f-strings. The printBoard function uses f-string syntax, and works properly, as long as you use a newer Python interpreter. \$\endgroup\$
    – AJNeufeld
    Sep 19, 2020 at 6:51
  • \$\begingroup\$ Thanks so much for the input! I am not sure which version of python you were using, but I am using python 3.8.5, which allows formatted strings, which was what I employed to print my board. I also now realize that I was lax in debugging my code properly, there were several errors in my original code, as you had indicated. Thanks! \$\endgroup\$
    – fartgeek
    Sep 21, 2020 at 1:35

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