10
\$\begingroup\$

Here is a game of tic-tac-toe I created to run in python 2.7. I am still learning and feel 400+ lines of code is a bit lengthy for such a small application. However, trying to get some feedback on how I could have put the game together better (and shorter), while still retaining full functionality.

Any and all feedback is welcome. Please note that I have only been working with Python for a few weeks so anything too advanced I may not be able to wrap my head around.

# Game of Tic-Tac-Toe
import os
import sys

def Start():
    # Variables
    a1 = "1"
    a2 = "2"
    a3 = "3"
    a4 = "4"
    a5 = "5"
    a6 = "6"
    a7 = "7"
    a8 = "8"
    a9 = "9"
    location = "0"

    # Splash Screen
    os.system('cls' if os.name == 'nt' else 'clear')
    print "Welcome to Tic-Tac-Toe!"
    print """

       |   |
    ---|---|---
       |   |
    ---|---|---
       |   |

    You may press CTRL+C at any time to quit this game.
    Beware that progress is not saved.

    Who shall go first, X or O?
    """
    start = raw_input("Please enter \"X\" or \"O\" when ready> ")

    if start == "X" or start == "x":
        Turn_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)
    elif start == "O" or start == "o":
        Turn_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)
    else:
        Start(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Turn_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    os.system('cls' if os.name == 'nt' else 'clear')
    print """

     %s | %s | %s
    ---|---|---
     %s | %s | %s
    ---|---|---
     %s | %s | %s

    """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    # Ask for play
    print "It is Player X\'s turn"
    location = raw_input("Please enter a location to play (1-9)> ")
    Place_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Place_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    if location == a1:
        a1 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a2:
        a2 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a3:
        a3 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a4:
        a4 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a5:
        a5 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a6:
        a6 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a7:
        a7 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a8:
        a8 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a9:
        a9 = "X"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    else:
        Bad_Entry_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

    Win_Check_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Bad_Entry_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    os.system('cls' if os.name == 'nt' else 'clear')
    print """

     %s | %s | %s
    ---|---|---
     %s | %s | %s
    ---|---|---
     %s | %s | %s

    """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    # Ask for play
    print "Player X chose %s." % location
    print "This is an invalid location."
    location = raw_input("Please enter a location for Player X to play (1-   9)> ")
    Place_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Win_Check_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    # Win checks
    if (a1 == "X" and a2 == "X" and a3 == "X"):
        a1 = "-"
        a2 = "-"
        a3 = "-"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a4 == "X" and a5 == "X" and a6 == "X"):
        a4 = "-"
        a5 = "-"
        a6 = "-"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a7 == "X" and a8 == "X" and a9 == "X"):
        a7 = "-"
        a8 = "-"
        a9 = "-"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a1 == "X" and a4 == "X" and a7 == "X"):
        a1 = "|"
        a4 = "|"
        a7 = "|"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a2 == "X" and a5 == "X" and a8 == "X"):
        a2 = "|"
        a5 = "|"
        a8 = "|"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a3 == "X" and a6 == "X" and a9 == "X"):
        a3 = "|"
        a6 = "|"
        a9 = "|"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a1 == "X" and a5 == "X" and a9 == "X"):
        a1 = "\\"
        a5 = "\\"
        a9 = "\\"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a3 == "X" and a5 == "X" and a7 == "X"):
        a3 = "/"
        a5 = "/"
        a7 = "/"
        Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    else:
        if a1 != "1" and a2 != "2" and a3 != "3" and a4 != "4" and a5 != "5" and a6 != "6" and a7 != "7" and a8 != "8" and a9 != "9":
            Win_T(a1, a2, a3, a4, a5, a6, a7, a8, a9)
        else:
            Turn_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Turn_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    os.system('cls' if os.name == 'nt' else 'clear')
    print """

     %s | %s | %s
    ---|---|---
     %s | %s | %s
    ---|---|---
     %s | %s | %s

    """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    # Ask for play
    print "It is Player O\'s turn"
    location = raw_input("Please enter a location to play (1-9)> ")
    Place_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Place_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    if location == a1:
        a1 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a2:
        a2 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a3:
        a3 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a4:
        a4 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a5:
        a5 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a6:
        a6 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a7:
        a7 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a8:
        a8 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    elif location == a9:
        a9 = "O"
        print """

         %s | %s | %s
        ---|---|---
         %s | %s | %s
        ---|---|---
         %s | %s | %s

        """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    else:
        Bad_Entry_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

    Win_Check_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Bad_Entry_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    os.system('cls' if os.name == 'nt' else 'clear')
    print """

     %s | %s | %s
    ---|---|---
     %s | %s | %s
    ---|---|---
     %s | %s | %s

    """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    # Ask for play
    print "Player O chose %s." % location
    print "This is an invalid location."
    location = raw_input("Please enter a location for Player O to play (1-9)> ")
    Place_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Win_Check_O(a1, a2, a3, a4, a5, a6, a7, a8, a9, location):
    # Win checks
    if (a1 == "O" and a2 == "O" and a3 == "O"):
        a1 = "-"
        a2 = "-"
        a3 = "-"
    Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a4 == "O" and a5 == "O" and a6 == "O"):
        a4 = "-"
        a5 = "-"
        a6 = "-"
        Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a7 == "O" and a8 == "O" and a9 == "O"):
        a7 = "-"
        a8 = "-"
        a9 = "-"
        Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a1 == "O" and a4 == "O" and a7 == "O"):
        a1 = "|"
        a4 = "|"
        a7 = "|"
        Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a2 == "O" and a5 == "O" and a8 == "O"):
        a2 = "|"
        a5 = "|"
        a8 = "|"
        Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a3 == "O" and a6 == "O" and a9 == "O"):
        a3 = "|"
        a6 = "|"
        a9 = "|"
        Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a1 == "O" and a5 == "O" and a9 == "O"):
        a1 = "\\"
        a5 = "\\"
        a9 = "\\"
        Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    if (a3 == "O" and a5 == "O" and a7 == "O"):
        a3 = "/"
        a5 = "/"
        a7 = "/"
        Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9)
    else:
        if a1 != "1" and a2 != "2" and a3 != "3" and a4 != "4" and a5 != "5" and a6 != "6" and a7 != "7" and a8 != "8" and a9 != "9":
            Win_T(a1, a2, a3, a4, a5, a6, a7, a8, a9)
        else:
            Turn_X(a1, a2, a3, a4, a5, a6, a7, a8, a9, location)

def Win_X(a1, a2, a3, a4, a5, a6, a7, a8, a9):
    os.system('cls' if os.name == 'nt' else 'clear')
    print "Player X Wins"
    print """
     %s | %s | %s
    ---|---|---
     %s | %s | %s
    ---|---|---
     %s | %s | %s

    """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    Replay()

def Win_O(a1, a2, a3, a4, a5, a6, a7, a8, a9):
    os.system('cls' if os.name == 'nt' else 'clear')
    print "Player O Wins"
    print """
     %s | %s | %s
    ---|---|---
     %s | %s | %s
    ---|---|---
     %s | %s | %s

    """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    Replay()

def Win_T(a1, a2, a3, a4, a5, a6, a7, a8, a9):
    os.system('cls' if os.name == 'nt' else 'clear')
    print "The game is a tied game."
    print "Congratulations! Noone has lost."
    print """
     %s | %s | %s
    ---|---|---
     %s | %s | %s
    ---|---|---
     %s | %s | %s

    """ % (a1, a2, a3, a4, a5, a6, a7, a8, a9)
    Replay()

def Replay():
    over = raw_input("Would you like to play again? (Y / N) ")
    if over == "Y" or over == "y":
        Start()
    elif over == "N" or over == "n":
        exit(0)
    else:
        Replay()

Start()
#Winning combos-
# (1 2 3)
# (4 5 6)
# (7 8 9)
# (1 4 7)
# (2 5 8)
# (3 6 9)
# (1 5 9)
# (3 5 7)
\$\endgroup\$
7
\$\begingroup\$

Trying to keep the same general approach, but improving the implementation:

  1. You define all the numbers in separate variables, and pass those variables around individually. It would be easier to define them in a list.
  2. Rather than manually creating the numbers, you can use range to generate a list of numbers. They don't need to be strings, python will convert them to strings when printing. To make it portable across python versions, use list(range()).
  3. You can use int to convert string versions of integers to numbers. So ind = int(location)-1 will convert the user's numerical input directly to a location in the list, so you don't need to compare elements of the list, not to mention individual variables.
  4. You can set locations in a list using the index, so board[ind] = 'x'.
  5. You can use in to match multiple values, such as if start in 'xY', if start == "X" or start == "x":. However, since you just want to do a case-insensitive match, you can start.upper() == 'X' to convert start to an uppercase value and then just match the uppercase version. Or better yet just do raw_input('foo').upper() to make it uppercase from the beginning.
  6. You have separate functions for handling x and o moves. It would be simpler to have one function that accepts the player as an argument.
  7. You can use .format(*board) to expand all the values in the list board to individual arguments to format.
  8. Rather than checking particular combinations manually, you can check particular combinations of indices that correspond to a "win" and check if those all have the same pieces.
  9. You can use a set to determine if the game is over. A set is kind of like a list, but it only allows one of each value, and doesn't preserve. So if you do set(board) == {'X', 'O'}, it will be true if and only if there are no numbers left on the board (that is, the game is over).
  10. You use the same code to create your board many times. It would be easier to split this out into its own function. This could include clearing the output.
  11. You wrap your start() line in if __name__ == '__main__': That way it will only run if the code is used as a function.
  12. You should use from __future__ import print_function and use print(). This will make your code portable across Python 2 and Python 3 and can also make it easier to print your board by using the sep argument.
  13. You run the code recursively. That is, function A calls function B which calls function C which calls function A again. it would be easier to keep track of the flow of the code using a loop. You can then switch players each time through the loop.
  14. In python you can define default values for function arguments. So you could make the board printing function print an empty board if no numbers are provided, or you can make it print extra text if another argument is provided.
  15. You don't check if a particular board position is already occupied.
  16. Don't use exit(0). Just let the script exit a function normally.
  17. Python3 has renamed raw_input to just input. You can make the code python 3 compatible by renaming raw_input to input if it exists.
  18. pep8 defines the normal style for python code. This includes capitalization, whitespace, etc. You don't need to follow it, but it is a general convention.

So, here is my version of your code. If you have any questions, feel free to ask:

"""Game of Tic-Tac-Toe"""

from __future__ import print_function

import os

# Make code work with Python 2 and Python 3
try:
    input = raw_input
except NameError:
    pass


WINS = [(0, 1, 2),
        (3, 4, 5),
        (6, 7, 8),
        (0, 3, 6),
        (1, 4, 7),
        (2, 5, 8),
        (0, 4, 8),
        (2, 4, 6)]


def start():
    board = list(range(1, 10))

    print_board(header="Welcome to Tic-Tac-Toe!")
    print('You may press CTRL+C at any time to quit this game.')
    print('Beware that progress is not saved.')
    print('')

    while True:
        rawplayer = input("Who shall go first \"X\" or \"O\"? > ")
        player = rawplayer.upper()
        if player in 'XO':
            break
        print('Invalid player', rawplayer)

    while set(board) != {'X', 'O'}:
        print_board(board, 'Player {} turn'.format(player))
        turn(board, player)
        if iswin(board, player):
            print_board(board, 'Player {} wins!'.format(player))
            return
        player = 'X' if player == 'O' else 'O'

    print_board(board, 'Game Over!')


def multirun():
    start()
    while True:
        over = input("Would you like to play again? (Y / N) ")
        if over.upper() == "Y":
            start()
        elif over.upper() == "N":
            return
        else:
            print('Invalid input', over)


def print_board(board=None, header=None):
    os.system('cls' if os.name == 'nt' else 'clear')
    if board is None:
        board = [' ']*9
    if header:
        print(header)
    print("""

     {} | {} | {}
    ---|---|---
     {} | {} | {}
    ---|---|---
     {} | {} | {}

    """.format(*board))


def turn(board, player):
    while True:
        location = input("Please enter a location to play (1-9) > ")
        try:
            ind = int(location)-1
        except ValueError:
            print('Invalid input', location)
            continue
        if ind < 0 or ind > 8:
            print('Location', location, 'out of range (1-9)')
            continue
        if board[ind] in ('X', 'O'):
            print('Location', location, 'already occupied')
            continue
        break
    board[ind] = player


def iswin(board, player):
    for win in WINS:
        if all(board[ind] == player for ind in win):
            return True


if __name__ == '__main__':
    multirun()
\$\endgroup\$
0
\$\begingroup\$

The long answer.

Directly when you feel that you are repeating yourself, you are on track, because then you know that there is a better solution. Thats the trickery of programming, to let the language do the work for you.

In your case this concerns the if statements, they are not needed in this manner. Do you think you could do it with one if statement? Certainly. To do this you'll need a loop of some sort. Since you in principal know how many values you have to work with, it suggest that you should use a for loop. If you don't know how many values you have to work with, it suggest you should while loop.

But this would not work in your code as of yet! Because now every singel one of your coordinates is specific to one variable. You don't have anything to iterate over.

When you have many values to work with, and perhaps they are of the same sort to you, you should compress them into some kinda container, which also gives you something to iterate over! Put them into a list, a dict, a set, depending on what sort of values you have and what you want to do with them.

I would suggest you keep working with your code, dismantle it, reassemble it and make it shorter, and shorter.

Trail by error will get you there.

\$\endgroup\$

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.