7
\$\begingroup\$

This is just a simple counter, i was able to code it in two different ways but i want to know which way is more efficient and which way is more readable. and which set of code is more "correct" in broad-manner of speaking. and if there is another entirely different set of code that does the same function ?

Game Rules

This is a simple game , at first it asks the user if they want to go right or left . if the user picks left ; the program ends and it displays the "You Win!" message. however if the user picks anything else other than left ( for example : right ) the program will start a counter 'z' and it will display a different message for each time the user chooses the wrong direction or input. i have set it to 4 different messages and then one last message that keeps playing on repeat until the user picks the correct direction.

however i feel like that there must be better code to do this. can you please help?

the first set of code :

#This is a simple game where if you pick left you win,
#but if you pick right you keep going through 4 different levels of
#'you lose' messages and then a last one keeps repeating in a loop.

z = 0
n = input ( "Go left or right? \n" )
while n not in ["left", "Left"]:
    if z == 0 :   
        print ("You lose * 0 ")
        z = z+1
        n = input ( "Go left or right? \n" )
    elif z == 1 :   
        print ("You lose * 1 ")
        z = z+1
        n = input ( "Go left or right? \n" )
    elif z == 2 :
        print ("You lose * 2 ")
        z = z+1
        n = input ( "Go left or right? \n" )
    elif z == 3 :
        print ("You lose * 3 ")
        z = " "
        n = input ( "Go left or right? \n" )
    else :
        while n not in ["left", "Left" ] :
            print ( "You Lose * Infinite" )
            n = input ( "Go left or right? \n" )
print ( "\nYou Win!" )

Second set of code :

#This is a simple game where if you pick left you win,
#but if you pick right you keep going through 4 diffrent levels of
#'you lose' messegses and then a last one keeps repeating on a loop.



v = 0
z = 0
u = 0
x = 0 
n = 0

while x == 0 :
    n = input ("Go Left or Right?")
    while n not in ["left", "Left"] and u == 0 :
        if   z == 0:
            print ( " You lose * 0 ")
        elif z == 1:
            print ( " You lose * 1 ")
        elif z == 2:
            print ( " You lose * 2 ")
        elif z == 3:
            print ( " You lose * 3 ")
        else :
            print ( " You lose * Infinite " )
        u = " "   
    while z < 4 and v == 0 :
        z = z + 1
        v = " "
    u = 0  
    v = 0
    if n == "left" or n == "Left":
        x=" "


print ( " You Win! ")

final set of code :

user = input ("Go left or right?")
counter = 0

while user not in ["left", "Left"] :
    if counter == 0 :
        print ("You lose * 0")
    elif counter == 1 :
        print ("You lose * 1")
    elif counter == 2 :
        print ("You lose * 2")
    elif counter == 3 :
        print ("You lose * 3")
    else :
        print ("You lose * Infinite")
    counter = counter + 1
    user = input ("Go left or right?")

print ("You Win!")
\$\endgroup\$
11
\$\begingroup\$

You're doing some weird things with the scripts. I consider the first to be better, since especially in the second one, you're doing a lot of things with variables which we have better ways to do. I'll point those out separately - lets first have a look at your first script.

Your first script

Code Duplication

If you find yourself typing the same thing a lot, you're probably doing something wrong. Examples of this are incrementing z and printing the message.

Variable Names

Variables should have short but meaningful names. That means that if you find yourself using variables like z or n, you're probably doing something wrong somewhere.

Loops and termination

We have a really nice command named break in python. It will terminate the inner-most loop. Using this will obsolete all nested loops your currently have.

I'll keep using while loops here, but you might also want to try the following instead:

from itertools import count

for iteration in count(1):
    # do stuff...
    if we_are_done():
        break

this function is basically an infinity list - it's doing exactly the same as:

iteration = 0
while True:
    iteration += 1
    # do stuff....
    if we_are_done():
        break

Which we'll be doing all the time here.

Keeping that all in mind....

iteration = 0
while True:
    command = input("Go Left or Right?")
    if "left" in command.lower():  # Transforms the input to all lowercase. This makes it case-insensitive.
        # We also use "in" as membership test, so that whitespace is ignored, and even an command like
        # "go left" will be considered valid. 
        break  # Terminates the loop.
    iteration += 1
    print(f"You lose * {iteration if iteration < 4 else 'infinity'}")
print("\nYou Win!")

This will do the same as your first script. In the loop, we get the input. We decide if we have won yet, and if we do, we break out. Then we increment the iteration variable. Then we print how often we've moved wrong before, or if it's more than 3 times, we instead print we lost * infinity.

If you're confused by the way we put our variable in that string, you should read up on f-strings. If you're confused by how we print that number, read up on ternary expressions. A good python programmer will know both. If the python version allows it, (s)he'll probably also use both.

Now lets dissect your second function...

I'll ignore everything I've already said something about regarding the first script.

Variable instantiation - don't do it unless you have to

v = 0
z = 0
u = 0
x = 0 
n = 0

If you find yourself writing something like this, you should ask yourself if you really need all those. Generally, you'll only instantiate variables in python when you need them, and leave them undefined as long as you don't. IF, and this is a big if, you really need them, you'd write it like this:

v = z = u = x = n = 0

How not to loop

Your outer loop should of course be either with count() or while True:. But the first inner loop is just obsolete alltogether. It cannot run more than once, since there's an u == 0 requirement for it to run, and at the end of the loop you unconditionally set it to " ", which is a totally different something. Your second inner loop does the exact same thing, but with v.

For both of these, you should at the least change the loops to if statements. However, if you also do the following paragraph, you can just remove them and unindent your code instead.

Do it just once if you can.

You check for n not in ["left", "Left"] twice. Of course, the first time is to guard against the printing, and the second is to break out of your loop. If you just break at the top, you'll have avoided having to do it twice.

\$\endgroup\$
  • 1
    \$\begingroup\$ "In both cases, you can just delete your loops and unindent your code" - This does not seem correct to me. It would be correct if the u==0 was the only condition of the loop, but since there is an additional condition he needs to replace the while with an if. \$\endgroup\$ – Taemyr Sep 21 at 6:31
  • \$\begingroup\$ thanks you both for your precious feedback. i did forget to mention that both code sets are functional and work without any errors. i also forgot to mention that i started working learning python 2 days ago so this was the best i could do with what i currently know, however i did try to improve my code. i'll add the new code to my question . if you think it's an improvement on the previous two, please say so ! \$\endgroup\$ – Elbasel Sep 21 at 6:35
  • \$\begingroup\$ As is, yes you'd need to put in if statements. However, if the loop termination is moved up as I suggested, that wouldn't be neccesary. I'll try and make that more clear. \$\endgroup\$ – Gloweye Sep 21 at 6:41
  • \$\begingroup\$ New one is better, but you're still having a lot of code duplication, especially in your switch statements. Compare it to the smaller script I have after my header "keeping that all in mind". It's good to get in the habit to avoid code duplication, since it will make bug-fixing much easier later on, since you'll only have to fix bugs in one place. \$\endgroup\$ – Gloweye Sep 21 at 6:47

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.