-1
\$\begingroup\$

Here is my code:

def winnerchecker():
    def player1win():
        print '\n' * 5, name, "YOU HAVE WON! GG TO", xplayer
        time.sleep(3)
        raise SystemExit()

    try:
        if "1,1" in turn1l and "1,2" in turn1l and "1,3"in turn1l:
            player1win()
        elif "1,1" in turn1l and "2,1" in turn1l and "3,1" in turn1l:
            player1win()
        elif "2,1" in turn1l and "2,2" in turn1l and "2,3" in turn1l:
            player1win()
        elif "3,1" in turn1l and "3,2" in turn1l and "3,3" in turn1l:
            player1win()
        elif "1,2" in turn1l and "2,2" in turn1l and "3,2" in turn1l:
            player1win()
        elif "1,3" in turn1l and "2,3" in turn1l and "3,3" in turn1l:
            player1win()
        elif "1,1" in turn1l and "2,2" in turn1l and "3,3" in turn1l:
            player1win()
    except NameError:
        pass

As you can see it is very long, is there anyway I could clean all this up into 1 if statement? Thank you

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review. Please tell us more about what you intend to accomplish with this code and what turn1l represents. We could mechanically transform this code, but it still wouldn't be good advice unless we understand your intentions. (See How to Ask.) \$\endgroup\$ Apr 9 '16 at 22:16
2
\$\begingroup\$

My initial question is: What is turn1l? Is it a set? A list? A string? The following is based on turn1l being a set/list.

Write

checks = [
    # Vertical (or horizontal, depending on your datastructure)
    set(["1,1", "1,2", "1,3"]),
    set(["2,1", "2,2", "2,3"]),
    set(["3,1", "3,2", "3,3"]),
    # Horizontal (or vertical, depending on your datastructure)
    set(["1,1", "2,1", "3,1"]),
    set(["1,2", "2,2", "3,2"]),
    set(["1,3", "2,3", "3,3"]),
    # Diagonals
    set(["1,1", "2,2", "3,3"]),
    set(["3,1", "2,2", "1,3"]),
]

Then, write

def winnerchecker():
    def player1win():
        print '\n' * 5, name, "YOU HAVE WON! GG TO", xplayer
        time.sleep(3)
        raise SystemExit()

    try:
        for check in checks:
            if check.issubset(turn1l):
                player1win()
    except NameError:
        pass

Now, you don't need player1win as a separate function anymore. Inline it.

def winnerchecker():
    try:
        for check in checks:
            if check.issubset(turn1l):
                print '\n' * 5, name, "YOU HAVE WON! GG TO", xplayer
                time.sleep(3)
                raise SystemExit()
    except NameError:
        pass

Now there are a few more suggestions I'd like to make:

  • Don't raise SystemExit. Prefer os.exit(), or better: write your code in such a way that the flow of control makes it stop, instead of having to call os.exit.
  • Don't catch NameError. It will hide bugs: name might be undefined, xplayer might be undefined, time might be undefined (not imported), or turn1l might not be defined yet.
  • Don't store strings in turn1l. Prefer tuples of ints. (1, 3) instead of "1,3". Or, store the state as a matrix of cells instead of a list of player1-entries and player2-entries.

Judging by the above two facts, I think the code you have written can be cleaned up a lot more, but you have not supplied it. If you do (in another question, please!), I'd be happy to take a look at the rest.

\$\endgroup\$
1
\$\begingroup\$

Perhaps try using a loop to test for the x and y values. Something like:

for i in range(1, 4):
    if i ,1 in turn1l and i,2 in turn1l and i,3 in turn1l:
         player1Win();
    if 1, i in turn1l and 2, i in turn1l and 3,i in turn1l:
         player1Win();
if "1,1" in turn1l and "2,2" in turn1l and "3,3" in turn1l:
        player1win()
elif "3,1" in turn1l and "2,2" in turn1l and "1,3" in turn1l:
        player1win()

Forgive me, I don't know python, so this might be a little off, but the logic works. The for loop allows you to test for Horizontals and Verticals. Then you just need to add the diagonals. Good luck!

\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$
    – SirPython
    Apr 9 '16 at 18:51

Not the answer you're looking for? Browse other questions tagged or ask your own question.