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
\$\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\$
    – Mast
    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\$
    – MrLok3rs
    Jan 23, 2020 at 11:16
  • \$\begingroup\$ Yes, that's it. \$\endgroup\$
    – Mast
    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\$
    – greybeard
    Jan 23, 2020 at 12:39
  • \$\begingroup\$ I'd edit post, what do you think about it now? \$\endgroup\$
    – MrLok3rs
    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 :)

\$\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\$
    – MrLok3rs
    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\$
    – Tlomoloko
    Jan 23, 2020 at 17:37

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.