1
\$\begingroup\$

I've recently started learning Python again after a long time away from it and doing MATLAB at university. Today, I spent some time working on a first little project of mine. Right now it works as it should minus ending the game when all the fields are full. I would love to get advice on how I could improve the code and would like to fix any bad habits early on in learning.

 mapA = [[" ", "|", " ", "|", " "],
 [" ", "|", " ", "|", " "],
 [" ", "|", " ", "|", " "]]
 def printMap():
 # prints out the map
 map = ["".join(mapA[0]),
 "".join(mapA[1]),
 "".join(mapA[2])]
 print(map[0])
 print(map[1])
 print(map[2])
 # makes a list of just the fields with x and o
 mapB = [map[0].split("|"),
 map[1].split("|"),
 map[2].split("|")]
 for x in range(3): # converts the board to numbers
 for y in range(3):
 if mapB[x][y] == "X":
 mapB[x][y] = 1
 elif mapB[x][y] == "O":
 mapB[x][y] = -1
 elif mapB[x][y] == " ":
 mapB[x][y] = 0
 for x in range(3):
 if abs(sum(mapB[x])) == 3: # checks for full rows
 return "END"
 elif abs(int(mapB[0][x]) + int(mapB[1][x]) + int(mapB[2][x])) == 3: # checks for full columns
 return "END"
 if abs(int(mapB[0][0]) + int(mapB[1][1]) + int(mapB[2][2])) == 3: # checks for full right diagonal
 return "END"
 elif abs(int(mapB[0][2]) + int(mapB[1][1]) + int(mapB[2][0])) == 3: # checks for full left diagonal
 return "END"
 counter = 0
 def choice(): # allows the user to choose O or X
 print("O or X?")
 symbol = input().lower() # case insensitive
 if symbol == "o":
 return ["O", "X"]
 elif symbol == "x":
 return ["X", "O"]
 else:
 print("Invalid symbol")
 return choice() # restarts the function if the user input neither O nor X
 def placement(symbol):
 print("Place " + symbol + " in form: rowNumber columnNumber")
 x = input()
 coordinates = x.split(" ")
 while len(coordinates) != 2 or coordinates[0].isnumeric() != True or coordinates[1].isnumeric() != True:
 print("Invalid input.")
 print("Place " + symbol + " in form: rowNumber columnNumber")
 x = input()
 coordinates = x.split(" ")
 while mapA[int(coordinates[0]) - 1][int(coordinates[1]) * 2 - 2] != " ":
 if mapA[int(coordinates[0]) - 1][int(coordinates[1]) * 2 - 2] != " ":
 print("That space is taken.")
 else:
 print("Invalid input.")
 print("Place " + symbol + " in form: rowNumber columnNumber")
 x = input()
 coordinates = x.split(" ")
 if coordinates[0] == "1" and coordinates[1] == "1":
 mapA[0][0] = symbol
 elif coordinates[0] == "1" and coordinates[1] == "2":
 mapA[0][2] = symbol
 elif coordinates[0] == "1" and coordinates[1] == "3":
 mapA[0][4] = symbol
 elif coordinates[0] == "2" and coordinates[1] == "1":
 mapA[1][0] = symbol
 elif coordinates[0] == "2" and coordinates[1] == "2":
 mapA[1][2] = symbol
 elif coordinates[0] == "2" and coordinates[1] == "3":
 mapA[1][4] = symbol
 elif coordinates[0] == "3" and coordinates[1] == "1":
 mapA[2][0] = symbol
 elif coordinates[0] == "3" and coordinates[1] == "2":
 mapA[2][2] = symbol
 elif coordinates[0] == "3" and coordinates[1] == "3":
 mapA[2][4] = symbol
 return printMap()
 symbol = choice()
 printMap()
 end = ""
 while end != "END":
 end = placement(symbol[0])
 if end == "END":
 print("Game Finished")
 break
 end = placement(symbol[1])
 if end == "END":
 print("Game Finished")
 break
tictac()
asked Mar 31, 2021 at 23:56
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to CodeReview. Here we can review complete and working code. If your application is not finished and you are struggling with some problem then please consider to ask question on StackOverflow. \$\endgroup\$ Commented Apr 1, 2021 at 6:37

1 Answer 1

2
\$\begingroup\$

The first thing I notice is the two separate maps. Since the only actual information you're storing is X / O positions in a 3x3 grid, I think it is good practice to only store those values. Then add any additional formatting when you print. I've also used snake_case for function and variable names, which is the preferred style. The first section could be rewritten:

x_o_grid = [
 [' ', ' ', ' ', ],
 [' ', ' ', ' ', ],
 [' ', ' ', ' ', ]
]
def print_map():
 for row in x_o_grid:
 print('|'.join(row))

You can continue with this pattern by again only storing -1, 0, or 1 in this grid. Then doing your comparison in a slightly different way.

I would suggest refactoring the portion of printMap where you check for matches into a new check_for_solution function:

def check_for_solution():
 # Check rows
 for row in x_o_grid:
 if all(x == row[0] for x in row):
 if row[0] != ' ':
 return row[0]
 # Check columns
 for i in range(len(x_o_grid)):
 column = [row[i] for row in x_o_grid]
 if all(x == column[0] for x in column):
 if column[0] != ' ':
 return column[0]
 # Check diagonals
 diagonal1 = [x_o_grid[i][i] for i in range(len(x_o_grid))]
 diagonal2 = [x_o_grid[len(x_o_grid) - i][len(x_o_grid) - i] for i in range(len(x_o_grid))]
 for diagonal in [diagonal1, diagonal2]:
 if all(x == diagonal[0] for x in diagonal):
 if diagonal[0] != ' ':
 return diagonal[0]
 return None

In the above rewrite, I have tried to avoid hardcoding the indices of the grid we are checking. And instead of using sum() to check for equivalence, I'm using the all() function where we check to see if the first element of the list is equal to every other element. This will return true if it is or false if any element is off.

This helps to make our code reusable and neater and allows for more flexible data storage. By not using range(3) and instead just looping through the available rows, we can accommodate any square matrix when checking for solutions.

The placement you can change != True to not. I believe the below will also error out if you pass a None or a float, so these should be captured in try: except: statements.

while not len(coordinates) == 2 or not coordinates[0].isnumeric() or not coordinates[1].isnumeric():

You might also consider f-strings, available in Python 3.6+. I believe this is preferred, but may come down to preference.

print(f"Place {symbol} in form: rowNumber columnNumber")

The placement logic can also be simplified to:

x_o_grid[int(coordinates[0]) - 1][int(coordinates[1]) - 1] = symbol

I would also recommend you change the end variable to a bool since you're only really checking for two states:

end = False
while not end:
 ...

I believe the check_solutions function will require a rewrite of your main loop, as it now returns either a none or the winning value.

Peilonrayz
44.4k7 gold badges80 silver badges157 bronze badges
answered Apr 1, 2021 at 17:31
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.