2
\$\begingroup\$

This is my first program. Its a two player noughts and crosses game. I would really appreciate any comments on how to improve my code or make it more efficient. Many Thanks.

It is written in Python 3.6.

#Noughts and Crosses Game

#Grid
grid = [0, 1, 2, 3, 4, 5, 6, 7, 8]

#Welcome
print("Welcome to Noughts and Crosses Game")

#Player name inputs
p1_name = input("What is name of Player 1? Player 1 is 'x': ")
p2_name = input("What is name of Player 2? Player 2 is 'o': ")

#Draw Grid
def draw_grid():
    print("-------------")
    print('|', grid[0], '|', grid[1], '|', grid[2], '|')
    print("-------------")
    print('|', grid[3], '|', grid[4], '|', grid[5], '|')
    print("-------------")
    print('|', grid[6], '|', grid[7], '|', grid[8], '|')
    print("-------------")

#Move input for player 1 
def p1():
        try:
            p1_input = int(input(p1_name + " select a space on the grid by entering a \
number to place an 'x': "))
        except ValueError:
            print("Error: Please enter a number, not a letter or symbol")
            p1()
        else:
            if p1_input not in range(9):
                print("Error: Please select a number on the grid 0-8 to place an 'x'")
                p1()
            elif grid[p1_input] == "x" or grid[p1_input] == "o":
                print("Error: This spot has been taken.")
                p1()
            else:
                grid[p1_input] = "x"
                draw_grid()

#Move input for player 2
def p2():
        try:
            p2_input = int(input(p2_name + " select a space on the grid by entering a \
number to place an 'o': "))
        except ValueError:
            print("Error: Please enter a number, not a letter or symbol")
            p2()
        else:
            if p2_input not in range(9):
                print("Error: Please select a number on the grid 0-8 to place an 'o'") 
                p2()
            elif grid[p2_input] == "x" or grid[p2_input] == "o":
                print("Error: This spot has been taken.")
                p2()
            else:
                grid[p2_input] = "o"
                draw_grid()

#Check for three x in a row
def p1_check():
    if grid[0] == "x" and grid[1] == "x" and grid[2] == "x" or \
        grid[3] == "x" and grid[4] == "x" and grid[5] == "x" or \
        grid[6] == "x" and grid[7] == "x" and grid[8] == "x" or \
        grid[0] == "x" and grid[3] == "x" and grid[6] == "x" or \
        grid[1] == "x" and grid[4] == "x" and grid[7] == "x" or \
        grid[2] == "x" and grid[5] == "x" and grid[8] == "x" or \
        grid[6] == "x" and grid[4] == "x" and grid[2] == "x" or \
        grid[0] == "x" and grid[4] == "x" and grid[8] == "x":
            print(p1_name + " Wins!")
            return "End Game"

#Check for three o in a row  
def p2_check():
    if grid[0] == "o" and grid[1] == "o" and grid[2] == "o" or \
         grid[3] == "o" and grid[4] == "o" and grid[5] == "o" or \
         grid[6] == "o" and grid[7] == "o" and grid[8] == "o" or \
         grid[0] == "o" and grid[3] == "o" and grid[6] == "o" or \
         grid[1] == "o" and grid[4] == "o" and grid[7] == "o" or \
         grid[2] == "o" and grid[5] == "o" and grid[8] == "o" or \
         grid[6] == "o" and grid[4] == "o" and grid[2] == "o" or \
         grid[0] == "o" and grid[4] == "o" and grid[8] == "o":
            print(p2_name + " Wins!")
            return "End Game"

#Check for a tie
def tie_check():
    if grid.count("x") + grid.count("o") == len(grid):
        print("Game is a tie")
        return "End Game"

#Play the game
def play():
    draw_grid()
    while True:
        p1()
        if p1_check() == "End Game":
            break
        if tie_check() == "End Game":
            break
        p2()
        if p2_check() == "End Game":
            break
        if tie_check() == "End Game":
            break

play()
\$\endgroup\$
1
\$\begingroup\$

Seems pretty nicely done. Most of the following are minor improvements, but if in case you do not get any of the following parts, do let me know. COnsidering the beginner tag, I have written a small appendix at the end. Suggestions for improvements:

  1. The Format method: Instead of using the print() as you have written, it is better to use a format() method in general. For your purposes, it might be overkill, but in case you are incorporating some of the points below (especially 5), it might come in handy for a pretty output.
  2. Alternative for if-elif-else: The entire block from try to the final else can be replaced by regexsearch (or any equivalent string search) to look for correct responses and club the not a number and incorrect number error messages into one. One suggestion for that is below.

Suggestion code:

# initialize with grid = ["0","1","2","3","4","5","6","7","8"]
if (p1_input not in grid) or (p1_input in ['x','o']):
    print('Combined Error Message')
else:
    grid[int(p1_input)]='x'
    draw_grid()
  1. Checking for victory: Instead of listing down all possible victory positions, the bulky code can be avoided. Although I am currently unable to come up with anything simpler than the following code which uses numpy, you can at least contract the current code in a single function that can be called for both p1 and p2, reducing the bulk of that code by half. The benefit of using the numpy variant is, it is size-independent. You can update a for a 4x4 game and it should be directly usable.

Numpy Variant:

def p1_check():
    # a defines a list of all winning positions
    a = [(0, 1, 2), (3, 4, 5), (6, 7, 8), (0, 3, 6), (1, 4, 7), (2, 5, 8), (0, 4, 8), (2, 4, 6)]
    # <= is the subset operator for sets. Additionally, the where statement finds the indices for all 'x'
    l = list(numpy.where(numpy.array(grid) == 'x')[0])
    if True in list(map(lambda x: (set(x) <= set(l)),a)):
        print('Victory')
        return "End Game"

Contraction of your code:

def check(player):
   a = ['x','o'][player-1]
   if grid[0] == a and grid[1] == a and grid[2] == a or \
         grid[3] == a and grid[4] == a and grid[5] == a or \
         grid[6] == a and grid[7] == a and grid[8] == a or \
         grid[0] == a and grid[3] == a and grid[6] == a or \
         grid[1] == a and grid[4] == a and grid[7] == a or \
         grid[2] == a and grid[5] == a and grid[8] == a or \
         grid[6] == a and grid[4] == a and grid[2] == a or \
         grid[0] == a and grid[4] == a and grid[8] == a:
            print([p1_name,p2_name][player-1] + " Wins!")
            return "End Game" 
#call check(1) and check(2) appropriately
  1. Earlier Ties: In order to account for configurations where a tie is definite, without all the boxes having been filled, you can work out if there is any winning chance considering only all the 'x' and the empty boxes (same for 'o'). To do this, one way is to use the numpy variant above with a slight modification. This matters very little in a 3x3 game but ismore relevant for larger games.

Suggestion code:

# a defines a list of all winning positions
def tie_check():
    a = [(0, 1, 2), (3, 4, 5), (6, 7, 8), (0, 3, 6), (1, 4, 7), (2, 5, 8), (0, 4, 8), (2, 4, 6)]
    # using != instead of ==, 'or' combining the lists
    canwin = (True in list(map(lambda x: set(x) <= set(list(numpy.where(numpy.array(grid) != 'x')[0])),a))) or \
          (True in list(map( lambda x: set(x) <= set(list(numpy.where(numpy.array(grid) != 'o')[0])),a)))
    if not canwin:
        print("Game is a tie")
        return "End Game"
  1. Multiple Games and statistics: To just improve the usability of the code, you can allow for specifying at the end whether to play another game or not. Provision for a best of 3/best of 5 can easily be incorporated in the play() function with an additional option of storing and printing the statistics for the game.

APPENDIX

Numpy is basically used for its arrays and array-methods which are very fast and convenient. For our purposes, only two of these are relevant. numpy.where just returns an array of indices corresponding to True. Whenever a numpy array is equated (==) to any value, an array of booleans is returned corresponding to element-wise matching. I urge you to try out with self-made arrays and it is very easy to get hang of, for 1-D case, which is the one used above.

Map is just a way to apply a function element-wise to a list and return a list of values returned for every such application. I assume you are familiar with lambda(a way to make functions on the fly) and set which are very similar to the mathematical entity.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Here's one small way I noticed to make your code more standard and easier to read: in your Numpy function example, instead of if True in list(map(lambda x: (set(x) <= set(l)),a)), you should simply do if any(map(lambda x: (set(x) <= set(l)), a)) \$\endgroup\$
    – Graham
    Nov 17 '18 at 13:20

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.