Tic-Tac-Toe Python 3 game

Question

I was given the challenge of creating a Tic-Tac-Toe game that would print out the board each time. It was fun to make and part B of the challenge was to have others review my code. If you would like to see my code in action, you can do so from here. Any comments or advice on adjustments I could apply to my program would be greatly appreciated.

import random

square_values = ["1", "2", "3", "4", "5", "6", "7", "8", "9"]
number_of_turns = 0
no_wins = True

print("Lets play Tic-Tac-Toe!")
player_1_pick = ""
player_2_pick = ""
player_1 = input("Enter a name for player 1 and press enter, leave blank to leave as Player 1: ")
player_2 = input("Enter a name for player 2 and press enter, leave blank to leave as Player 2: ")

#sets the players name
if (player_1 == "" or player_2 == ""):
  if (player_1 == ""):
    player_1 = "Player 1"
  if (player_2 == ""):
    player_2 = "Player 2"
else:
  pass

#assigns X or O to players
if (random.randint(1,2) == 1):
  player_1_pick = input(player_1 + ", choose X or O: ").upper()
  if (player_1_pick == "X"):
    player_2_pick = "O"
  else:
    player_2_pick = "X"
else:
  player_2_pick = input(player_2 + ", choose X or O: ").upper()
  if (player_2_pick == "X"):
    player_1_pick = "O"
  else:
    player_1_pick = "X"

#makes a move
def make_a_move(player, player_pick):
  print("""
     |     |     
  {}  |  {}  |  {}
_____|_____|_____
     |     |     
  {}  |  {}  |  {}   
_____|_____|_____
     |     |     
  {}  |  {}  |  {}  
     |     |
""" .format(square_values[0], square_values[1], square_values[2], square_values[3], square_values[4], square_values[5], square_values[6], square_values[7], square_values[8]))

  status = True
  while (status == True):
    choice = input(player + " pick a square(" + player_pick + "): ")
    try:
      int(choice)
      if (1 <= int(choice) <= 9):
        if (square_values[int(choice)-1] != "X" and square_values[int(choice)-1] != "O"):
          square_values.remove(choice)
          square_values.insert(int(choice)-1, player_pick)
          status = False
        else:
          print("Square already taken, select another square.")
      else:
        print("Input not an option, choose again.")
    except ValueError:
        print("Input not an option, choose again.")

status_main = True
def check_for_a_win(value1, value2, value3):
  global status_main
  global no_wins
  if (square_values[value1] == "X" and square_values[value2] == "X" and square_values[value3] == "X"):
    status_main = False
    no_wins = False
    if(player_1_pick == "X"):
      print("Player 1 won!")
    else:
      print("Player 2 won!")
  elif (square_values[value1] == "O" and square_values[value2] == "O" and square_values[value3] == "O"):
    status_main = False
    no_wins = False
    if(player_1_pick == "O"):
      print("Player 1 won!")
    else:
      print("Player 2 won!")
  else:
    pass

def func_1(player, pick):
  global number_of_turns
  global status_main
  if (no_wins == True):
    number_of_turns = number_of_turns + 1
    make_a_move(player, pick)
    check_for_a_win(0, 1, 2)
    check_for_a_win(3, 4, 5)
    check_for_a_win(6, 7, 8)
    check_for_a_win(0, 3, 6)
    check_for_a_win(1, 4, 7)
    check_for_a_win(2, 5, 8)
    check_for_a_win(0, 4, 8)
    check_for_a_win(2, 4, 6)
  if (number_of_turns == 9 and status_main == True):
    print("It's a tie :(")
    status_main = False

while (status_main == True):
  func_1(player_1, player_1_pick)
  func_1(player_2, player_2_pick)

Answer 1

Bug: If, during the "pick a sigil" part of the game setup, player 1 simply presses <ENTER>, the game will play empty-string vs. "X". It looks bad.

Your code has several problems. Let's start with the most basic:

1. You are not using Python idiom for structuring your code. Please see this page (one of many duckduck search results) for an example and explanation. You should be using:

   if __name__ == '__main__':
       play_game()
   

   (for some value of play_game().)

2. You are not using descriptive names for your functions. What does func_1 do?

3. You are not using functions to organize your code. From about line 7 down to the definition of make_a_move you have a bunch of code that is just executed in-line. That should be a setup_game() function, or some such.

4. Your logic is confused. Consider this code:

   #sets the players name
   if (player_1 == "" or player_2 == ""):
     if (player_1 == ""):
       player_1 = "Player 1"
     if (player_2 == ""):
       player_2 = "Player 2"
   else:
     pass
   

   What are the real possibilities here? If player one's name is empty, you need to set it:

   if (player_1 == ""):
       player_1 = "Player 1"
   

   Likewise for player 2:

   if (player_2 == ""):
       player_2 = "Player 2"
   

   All the rest of that code is just noise.

5. You are not using standard Python functions. Consider:

   square_values = ["1", "2", "3", "4", "5", "6", "7", "8", "9"]
   

   Is there a standard Python function that returns a range of values? Why, yes! There is!

   square_values = range(1, 10) # integers, not strings
   # or
   square_values = map(str, range(1, 10))
   # or 
   square_values = [str(n) for n in range(1, 10)]
   

6. Not a code problem, but a logic problem: why use player 1 and player 2? You ask for their names, and then the sigil they should use. Why not simply call them 'Player X' and 'Player O' all the way through? You could eliminate a lot of code that way.

7. Your code does not have "basic elegance" of structure. This is hard to quantify, but I would say that basic elegance requires that your control structures be used in as natural a fashion as possible. Consider this:

   while (status_main == True):
         func_1(player_1, player_1_pick)
         func_1(player_2, player_2_pick)
   

   This fails on many levels. First, comparing a boolean with True in a conditional expression is stupid. It's a boolean! Just use it:

   while (status_main):
   

   Next, using parens like this is a Java/C thing. Not required, nor recommended, in Python:

   while status_main:
       func_1(player_1, player_1_pick)
       func_1(player_2, player_2_pick)
   

   Next, what does status_main even mean? How about keep_playing instead?

   while keep_playing:
       func_1(player_1, player_1_pick)
       func_1(player_2, player_2_pick)
   

   But honestly, why not do the checking in the while loop, instead of relying on a global variable set who-knows-where?

   while not game_over():
       func_1(player_1, player_1_pick)
       func_1(player_2, player_2_pick)
   

   And, since all you are doing is alternating between player X and player O, why not include that in the loop. You can use that to insert a check for a win after each turn:

   curr, next = 'O', 'X'
   
   while not game_over():
       curr, next = next, curr
       take_turn(curr)
   
   print("Winner is", curr)  # Note that winner is in curr, if winner
   

   There are other problems like this. Look for your no_wins variable: almost every place it gets used looks inelegant. And calling a function won't help - you really need to think how to get rid of that variable. Here's hint: simplify your code. Move some logic out of the "bottom" functions and up into the "middle" and "top" functions.

Answer 2

I like your use of dictionary, I did this once but I followed the guide from this which is focusing more on AI. However, you can implement the "Play Again?" feature.

Keeping your code true to what it does my only real suggestion is instead of saying "Player 1 wins" try

print(player_1+' wins!')