Tic Tac Toe Shared Keyboard Game

Question

I wrote a Tic-Tac-Toe game for two players, where each player uses the same keyboard. It's not a school project or anything, I just thought this would be fun to code for practice.

I'm open to and looking for any sort of feedback. Do the comments contain sufficient detail to understand the code? Are the functions separated into their individual mechanisms/purposes properly? Is it, overall, a decently written algorithm?

import random
#initialization
board_arr = [1, 2, 3, 4, 5, 6, 7, 8, 9]
users = ['X', 'O']
user = random.choice(users)
winner = None
def print_board_horizontal():
 #print horizontal line (part of print board functions)
 print("-+-+-")
def print_board_numbered(b):
 """Print board_arr with slots numbered
 b: board array
 """
 print("\n"*3) #buffer lines
 print(str(b[0]) + "|" + str(b[1]) + "|" + str(b[2]))
 print_board_horizontal()
 print(str(b[3]) + "|" + str(b[4]) + "|" + str(b[5]))
 print_board_horizontal()
 print(str(b[6]) + "|" + str(b[7]) + "|" + str(b[8]))
 print("\n"*3) #buffer lines
def print_board(b):
 """Print board_arr
 b: board array
 """
 m = purge_board_of_int(b)
 print("\n"*3) #buffer lines
 print(str(m[0]) + "|" + str(m[1]) + "|" + str(m[2]))
 print_board_horizontal()
 print(str(m[3]) + "|" + str(m[4]) + "|" + str(m[5]))
 print_board_horizontal()
 print(str(m[6]) + "|" + str(m[7]) + "|" + str(m[8]))
 print("\n"*3) #buffer lines
def purge_board_of_int(b):
 """returns copy of board list with all integers replaced with empty spaces; returns list
 b: board array
 """
 _copy = b.copy()
 for e in range(len(_copy)):
 if type(_copy[e]) == int:
 _copy[e] = " "
 return _copy
def user_turn(b, u):
 """Reads user's move input and modified the game board list appropriately
 
 b: board list
 u: user
 """
 print("It is", u + "'s turn")
 #prompt user
 slot = -1
 while type(slot) is not int or slot < 0 or slot >= len(b) or type(b[slot]) == str:
 try:
 slot = int(input("Type a numbered slot: "))-1
 
 if slot < 0 or slot >= len(b) or type(b[slot]) == str:
 print("Invalid input")
 except:
 print("Invalid input")
 #update board
 b[slot] = u
def switch_turn(u):
 """Change current user turn (O or X); returns string of user
 
 u: user
 """
 if u == "O":
 return "X"
 else:
 return "O"
 
def check_winner(b, u):
 """Check if there is currently a winner; returns string of user (O or X)
 
 b: board array
 u: user
 """
 grid = len(b) // 3
 #check verticals, if 3 in a row (covered==3) current user wins
 for x in range(grid):
 covered = 0
 for y in range(x, len(b), grid):
 if b[y] == u:
 covered += 1
 if covered >= grid:
 return u
 #check horizontals, if 3 in a row (covered==3) current user wins
 for y in range(0, len(b), grid):
 covered = 0
 for x in range(y, y+grid, 1):
 if b[x] == u:
 covered += 1
 if covered >= grid:
 return u
 #diagonal from left, if 3 in a row (covered==3) current user wins
 covered = 0
 for i in range(0, 9, 4):
 if b[i] == u:
 covered += 1
 if covered >= grid:
 return u
 #diagonal from right, if 3 in a row (covered==3) current user wins
 covered = 0
 for i in range(2, 7, 2):
 if b[i] == u:
 covered += 1
 if covered >= grid:
 return u
#main loop
while winner == None:
 print_board_numbered(board_arr) #print numbered board
 print_board(board_arr) #print board strictly containing user input
 user_turn(board_arr, user) #prompt player input and modify board
 user = switch_turn(user) #switch user's turn
 winner = check_winner(board_arr, switch_turn(user)) #check if last user's input won
#win whooo!!
print_board(board_arr)
print(winner, "wins!")
```

• Tomerikoo: @Billal why did you remove the import? Even though just once, it is still being used...
• Billal BEGUERADJ: Because it can be subject to code review, just as code formatting is. But I accepted your other part of the review
• fartgeek: Use f-strings, my friend.
• Sᴀᴍ Onᴇᴌᴀ: Welcome to Code Review! Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see What should I do when someone answers my question? as well as what you may and may not do after receiving answers.
• Sᴀᴍ Onᴇᴌᴀ: @BillalBegueradj That import was in the original revision, on the same line as the opening code fence (where the language specifier can be). I have restored this, lest the code be seen as incomplete.

Answer 1

What's done well:

• Your function names are nice and fairly descriptive.
• Besides a few things (below), your formatting is good and consistent.
• You're following proper naming conventions
• You have doc-strings that give a basic idea of what each function does.

What can be improved:

• You have an odd mix of good names, and poor, single-character names. All your parameter names should have descriptive names. Good parameter names are important since they're implicitly a part of the function's documentation. When someone ctrl+q's your function, the parameter names, along with the doc-string, are what will show up. They should be thoroughly descriptive.

• You're only using 2-space indentation. PEP8 recommends 4-spaces though, and I think you'll find that it's much easier to read anyway. More indentation makes it easier to differentiate between levels at a glance, which becomes increasingly helpful as the code gets longer.

• Lines like this:

  print(str(m[0]) + "|" + str(m[1]) + "|" + str(m[2]))
  

  Can be neatened by making use of f-strings:

  print(f"{m[0]}|{m[1]}|{m[2]}")
  

  or, by just using print's capabilities directly, along with unpacking and slicing:

  print(*m[0:3], sep="|")
  print(*m[3:6], sep="|")
  print(*m[6:9], sep="|")
  

• purge_board_of_int can be greatly cleaned up:

  def purge_board_of_int(b):
   return [" " if isinstance(e, int) else e for e in b]
  

  The list comprehension produces a new list, so an explicit copy isn't necessary. From there, you can just pick what elements are put into the copy as it's being produced, instead of doing a full copy, then iterating it again to alter the elements.

• switch_turn is also fairly verbose. It can be simply:

  def switch_turn(u):
   return "X" if u == "O" else "O"
  

• except:: Never, ever, ever (unless you really know what you're doing), should you ever have a bare except like that. Never. It seems harmless until you accidentally introduce an error like an accidental KeyError inside the try block. A bare except: will catch all errors, even ones that you didn't intend for it to catch. When using trys, a couple good rules to follow are:

  • The block inside of the try should wrap as few lines as possible. Only include in the try what you expect to throw, or would otherwise be infeasible to factor out.

  • The except should specify exactly the subclass of Exception that you expect to catch. This requires some thought, but is very important. try should be used to handle only the errors that you expect to be thrown, and are able to handle gracefully. If you aren't expecting it, or you aren't able to properly recover from it, you should not catch it*. Doing so will just cover up bugs, and lead to very difficult debugging down the road. If you're using that try to catch ValueErrors thrown by int, specify that:

    except ValueError:
    

  I'm stating this so strongly because I have been bitten many times by errors being swallowed. It's also extremely common on Stack Overflow to see people post questions along the lines of "Help, my code's broken but I'm not getting any errors", and it turns out they have a try swallowing all helpful debugging information. Debugging without errors to help you is a hell I wouldn't wish on anyone. Put the thought in up front to save yourself headaches later.

  * Except for cases where you want to log all unknown errors, crashing the entire program wouldn't be appropriate, and when you're able to safely restart/bypass whatever had failed.

• while winner == None:: It took me a second to realize how winner would ever be None after the first iteration. You're relying on the implicit returning of None, which isn't a good idea. A good rule of thumb: if you explicitly return a value inside of a function, you should not rely on implicit return None. Why? When I'm reading your code, I want it to be clear what is returned in what case. There is no return None in that function though, so I'm left guessing "Did they accidentally miss potential case, and the None being returned is a bug, or is it intentional?". Make it clear that None is a valid value to be returned from that function by putting an explicit return None is the exact case that you want to return None. Don't make your reader guess, and don't allow yourself to forget that you're relying on that behavior when you inevitably modify the function later.

• The more common way of checking the type of an object is: isinstance(obj, SomeType). I actually don't know off-hand if it has any benefits over reference equality checks on the type using is, but I personally find it more readable.

An oddity:

• while type(slot) is not int . . .:: I can't see how slot would ever not be an int. It starts as -1 (an int), and the only other place it's assigned is slot = int(..., which means it must be given an int there as well. That checks seems to be useless.