HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Game of Tic-Tac-Toe

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereviewstackoverflowbeginnerpythonpython-2.xtic-tac-toe
tacticgametoe

Problem

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
---|---|---

Solution

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

  • You define all the numbers in separate variables, and pass those variables around individually. It would be easier to define them in a list.



  • 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()).



  • 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.



  • You can set locations in a list using the index, so board[ind] = 'x'.



  • 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.



  • 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.



  • You can use .format(*board) to expand all the values in the list board to individual arguments to format.



  • 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.



  • 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).



  • 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.



  • You wrap your start() line in if __name__ == '__main__': That way it will only run if the code is used as a function.



  • 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.



  • 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.



  • 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.



  • You don't check if a particular board position is already occupied.



  • Don't use exit(0). Just let the script exit a function normally.



  • 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.



  • 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("""

{}

Code Snippets

"""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()

Context

StackExchange Code Review Q#143321, answer score: 8

Revisions (0)

No revisions yet.