5
\$\begingroup\$

Can you provide some advice on how I can simplify this code?

I am a beginner with Python, and I'm pretty sure that my code can be much simpler. I think the check() function is far too complex. But I have no idea how to simplify it, and still check which player has won the game.

def check():
 win = 0
 if gameTable[0][0] == gameTable[0][1] == gameTable[0][2] == 'X':
 who(1)
 return True
 elif gameTable[1][0] == gameTable[1][1] == gameTable[1][2] == 'X':
 who(1)
 return True
 elif gameTable[2][0] == gameTable[2][1] == gameTable[2][2] == 'X':
 who(1)
 return True
 elif gameTable[0][0] == gameTable[1][0] == gameTable[2][0] == 'X':
 who(1)
 return True
 elif gameTable[0][1] == gameTable[1][1] == gameTable[2][1] == 'X':
 who(1)
 return True
 elif gameTable[0][2] == gameTable[1][2] == gameTable[2][2] == 'X':
 who(1)
 return True
 elif gameTable[0][0] == gameTable[1][1] == gameTable[2][2] == 'X':
 who(1)
 return True
 elif gameTable[0][2] == gameTable[1][1] == gameTable[2][0] == 'X':
 who(1)
 return True
 elif gameTable[0][0] == gameTable[0][1] == gameTable[0][2] == 'O':
 who(2)
 return True
 elif gameTable[1][0] == gameTable[1][1] == gameTable[1][2] == 'O':
 who(2)
 return True
 elif gameTable[2][0] == gameTable[2][1] == gameTable[2][2] == 'O':
 who(2)
 return True
 elif gameTable[0][0] == gameTable[1][0] == gameTable[2][0] == 'O':
 who(2)
 return True
 elif gameTable[0][1] == gameTable[1][1] == gameTable[2][1] == 'O':
 who(2)
 return True
 elif gameTable[0][2] == gameTable[1][2] == gameTable[2][2] == 'O':
 who(2)
 return True
 elif gameTable[0][0] == gameTable[1][1] == gameTable[2][2] == 'O':
 who(2)
 return True
 elif gameTable[0][2] == gameTable[1][1] == gameTable[2][0] == 'O':
 who(2)
 return True
 elif '-' not in gameTable[0] and '-' not in gameTable[1] and '-' not in gameTable[2]:
 who(3)
 return True
 else:
 return False
def who(win):
 if win == 1:
 print('\t---=== PLAYER 1 WINS ===---')
 if win == 2:
 print('\t---=== PLAYER 2 WINS ===---')
 if win == 3:
 print('\t---=== DRAW ===---')
 else:
 pass
def x_move():
 while True:
 x = input("Player {}\n Type coordinates of your move! (x, y):\n".format(p1))
 tabx = x.split(',')
 try:
 tabx[0] = int(tabx[0])-1
 tabx[1] = int(tabx[1])-1
 except:
 print("--==You've type invalid coordinates==--")
 continue
 try:
 if gameTable[tabx[0]][tabx[1]] == '-':
 gameTable[tabx[0]][tabx[1]] = 'X'
 print('\t', gameTable[0], '\n\t', gameTable[1], '\n\t', gameTable[2])
 break
 else:
 print("--==This place is already filled. Choose another.==--")
 except:
 print("--==You've type invalid coordinates==--")
 continue
def y_move():
 while True:
 y = input("Player {}\n Type coordinates of your move! (x, y):\n".format(p2))
 taby = y.split(',')
 try:
 taby[0] = int(taby[0])-1
 taby[1] = int(taby[1])-1
 except:
 print("--==You've type invalid coordinates==--")
 continue
 try:
 if gameTable[taby[0]][taby[1]] == '-':
 gameTable[taby[0]][taby[1]] = 'O'
 print('\t', gameTable[0], '\n\t', gameTable[1], '\n\t', gameTable[2])
 break
 else:
 print("--==This place is already filled. Choose another.==--")
 except:
 print("--==You've type invalid coordinates==--")
 continue
def game():
 print('\t', gameTable[0], '\n\t', gameTable[1], '\n\t', gameTable[2])
 while True:
 x_move()
 if check():
 break
 y_move()
 if check():
 break
p1 = input('First player name: ')
p2 = input('Second player name: ')
gameTable = [['-', '-', '-'],
 ['-', '-', '-'],
 ['-', '-', '-']]
game()

Ok, I changed that a little bit. I think now it looks better, but maybe is there anything more to make it simplier?

def check():
 win = 0
 if gameTable[0][0] == gameTable[0][1] == gameTable[0][2] != '-':
 who(1, gameTable[0][1])
 return True
 elif gameTable[1][0] == gameTable[1][1] == gameTable[1][2] != '-':
 who(1, gameTable[1][1])
 return True
 elif gameTable[2][0] == gameTable[2][1] == gameTable[2][2] != '-':
 who(1, gameTable[2][1])
 return True
 elif gameTable[0][0] == gameTable[1][0] == gameTable[2][0] != '-':
 who(1, gameTable[1][0])
 return True
 elif gameTable[0][1] == gameTable[1][1] == gameTable[2][1] != '-':
 who(1, gameTable[1][1])
 return True
 elif gameTable[0][2] == gameTable[1][2] == gameTable[2][2] != '-':
 who(1, gameTable[1][2])
 return True
 elif gameTable[0][0] == gameTable[1][1] == gameTable[2][2] != '-':
 who(1, gameTable[1][1])
 return True
 elif gameTable[0][2] == gameTable[1][1] == gameTable[2][0] != '-':
 who(1, gameTable[1][1])
 return True
 elif '-' not in gameTable[0] and '-' not in gameTable[1] and '-' not in gameTable[2]:
 who(2, '-')
 return True
 else:
 return False
def who(win, cords):
 if win == 1:
 if cords == 'X':
 print('---=== {} WINS A GAME ===---'.format(p1))
 elif cords == 'O':
 print('---=== {} WINS A GAME ===---'.format(p2))
 elif win == 2:
 print('\t---=== DRAW ===---')
 else:
 pass
Mast
13.8k12 gold badges57 silver badges127 bronze badges
asked Jan 23, 2020 at 10:33
\$\endgroup\$
7
  • \$\begingroup\$ Is there a reason at one point you have player 1 and 2 while at another you're referring to x_move and y_move? \$\endgroup\$ Commented Jan 23, 2020 at 10:58
  • \$\begingroup\$ I used p1 and p2 variables just for displaying name of players, is that what you meant? I'm sorry, english isn't my first language and I could not get your question \$\endgroup\$ Commented Jan 23, 2020 at 11:16
  • \$\begingroup\$ Yes, that's it. \$\endgroup\$ Commented Jan 23, 2020 at 12:05
  • \$\begingroup\$ There are different starting points for a quest for a simple solution. Starting from existing code, look for things that are the same between chunks (statements, sequences of statements, procedures, ...), and for things that differ. Visualise execution and look for the same things; additionally, for chances missed to re-use results. \$\endgroup\$ Commented Jan 23, 2020 at 12:39
  • \$\begingroup\$ I'd edit post, what do you think about it now? \$\endgroup\$ Commented Jan 23, 2020 at 14:17

1 Answer 1

3
\$\begingroup\$

Ok, so I'm also a beginner and some weeks ago I worked on a tic-tac-toe kind of game and learned a lot from it, here's what I think you could improve on.

DRY (Don't Repeat Yourself): Whenever you see that you are having to copy and paste your code a lot, you can generally assume there's a shorter way to do the same thing you're trying to do, either by using lists, for-loops, while-loops, etc.

One thing you can learn is using list comprehensions and the 'all' and 'any' operations, that's very useful when you are using lists.

players = ['X', 'O']
gameRows = [[gameTable[0][0], gameTable[0][1], gameTable[0][2]],
 [gameTable[1][0], gameTable[1][1], gameTable[1][2]],
 [gameTable[2][0], gameTable[2][1], gameTable[2][2]]]
def check():
 # You don't need to repeat the same code for both players,
 # Just use a for loop for each of them
 for player in players:
 # You should iterate all the columns and diagonals of the table
 for row in gameRows:
 # Returns True if all of the tiles in the row are equal the player
 if all(i == player for i in row):
 who(1, player)
 return True
 # Here you can add the 'gameTable' into a list
 if '-' not in gameTable[0] + gameTable[1] + gameTable[2]:
 who(2, '-')
 return True
def who(win, cords):
 # Here it's pretty much the same, instead of printing 'p1'
 # you could just print 'cords'
 if win == 1:
 if cords == 'X':
 print('---=== {} WINS A GAME ===---'.format(p1))
 elif cords == 'O':
 print('---=== {} WINS A GAME ===---'.format(p2))
 elif win == 2:
 print('\t---=== DRAW ===---')
 else:
 pass

You could have a 'gameColumns' and 'gameDiagonals' lists for you to iterate through them. Idk if this would actually work on your code, since you haven't provided all of it, but I think the general idea could be implemented. You could also check this link: https://www.python.org/dev/peps/pep-0008/ It's a style guide for writing code that is mostly accepted by all programmers, there's nothing wrong with using camelCase, it's just that people are generally used to other ways of casing variables. Hope you understood everything. Good luck on learning python :)

answered Jan 23, 2020 at 16:50
\$\endgroup\$
2
  • \$\begingroup\$ Well. when I just copied and pasted your code , it keeps giving me a draw after first move. Second thing is that your logic is checking if there are same characters in a row, what isn't a point of tic tac toe, where you can win typing your chars diagonally \$\endgroup\$ Commented Jan 23, 2020 at 17:26
  • \$\begingroup\$ Yeah, for checking diagonals you should have a list of the tiles characters in the diagonals too, that way you check for rows, columns and diagonals. About the tie issue, I edited my code so that it fixes the problem \$\endgroup\$ Commented Jan 23, 2020 at 17:37

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.