Beginner question

[Joshua Landau]

On 4 June 2013 04:39,  <eschneider92 at comcast.net> wrote:
> Is there a more efficient way of doing this? Any help is gratly appreciated.
>
>
> import random
> def partdeux():
>     print('''A man lunges at you with a knife!
> Do you DUCK or PARRY?''')
>     option1=('duck')
>     option2=('parry')
>     optionsindex=[option1, option2]
>     randomizer=random.choice(optionsindex)
>     while randomizer==option1:
>         if input() in option1:
>             print('he tumbles over you')
>             break
>         else:
>             print('he stabs you')
>             break
>     while randomizer==option2:
>         if input() in option2:
>             print('you trip him up')
>             break
>         else:
>             print('he stabs you')
>             break
> partdeux()

I'm going to look directly at the code for my comment, here, and
explain what's up. I imagine you were given this code to "fix up",
I'll lead you through the steps.



> import random

You only use "random.choice", never anything else, so in this case I'd
be tempted to do:
> from random import choice

This is *entirely optional*: I tend to do quite a lot of "from
<module> import <object>" but others prefer to be more explicit about
where things come from; your choice.



> def partdeux():

Other than the atrocious name this is as simple as it gets to define a
function - you should like that it's a function, though.



>     print('''A man lunges at you with a knife!
> Do you DUCK or PARRY?''')

This is iffy! Triple-quotes? Personally this is a really bad time to
use them - they break indentation and whatnot. I'd write:

>    print('A man lunges at you with a knife!')
>    print('Do you DUCK or PARRY?')

This, in my opinion, is much nicer. But we haven't "simplified" much yet.



>     option1=('duck')
>     option2=('parry')
>     optionsindex=[option1, option2]

There are two things off about this. Firstly, no-one should be so
addicted to brackets to use them here, and you should space variables.
>     option1 = 'duck'
>     option2 = 'parry'

BUT this is not needed anyway. The next line puts both of these in a
variable. You can add them straight in:
>     optionsindex = ['duck', 'parry']

There are some things wrong though:
1) *Never* lie. This is not an "index" of any sort, unless you're
referring to one of these:
[http://shop.pageprotectors.com/images/Index-Ring-Binder-2-Rings-Recipe-3x5-Card.jpg]
This should be named "options" or, better, "valid_responses".

2) You write "Do you DUCK or PARRY?". None of this suggests uppercase.
We shall deal with this later.

Thus:
>     valid_responses = ['duck', 'parry']


>     randomizer=random.choice(optionsindex)
>     while randomizer==option1:
...
>     while randomizer==option2:

This is odd! What do you think this does? This says that you choose an
option, "duck" or "parry". If it is "duck", then do A. If it is
"parry", then do B. But why would a computer choose options like that?
Surely it's better to do:

(I'm properly spacing it as I go along, by the way)
>    randomizer = random.choice([True, False])
>    while randomizer:
...
>    while not randomizer:

Oh, that's odd! As randomizer never changes after you set it, you can
be sure that the "while randomizer" is equivalent to "while True" or
"while False". This means it will loop forever without a break.
Looking at the breaks it is clear that *all paths lead to a break*. A
while *LOOP* should only ever be used to *LOOP*. This makes the
looping redundant.

Thus remove the breaks and use:
>    randomizer = random.choice([True, False])
>    if randomizer:
...
>    if not randomizer:

which can be better written:
>    if random.choice([True, False]):
...
>    else:
(BUT you may have to write "if choice([True, False]):" if you've
followed all of my advice)


>         if input() in option1:
"option1" no longer exists, so this is now written:
>         if input() in valid_responses[0]:
BUT why "in"? You want to check whether that *equals* the response,
not whether that is *in* the response:
>         if input() == valid_responses[0]:


>         else:
>             print('he stabs you')
Why "else"? This means that if you wrote "Seppuku" *he'd* stab you.
You want to check that you actually wrote the right thing!
>         elif input() == valid_responses[1]:
>             print('he stabs you')
This does not work, but we'll fix it later.


>         if input() in option2:
For the same reason, change this to:
>        if input() == valid_responses[1]:


>         else:
>             print('he stabs you')
and this to:
>         elif input() == valid_responses[0]:
>             print('he stabs you')


The rest is better. That leaves you with a much nicer looking function:

### START CODE ###
from random import choice

def partdeux():
    print("A man lunges at you with a knife!")
    print("Do you DUCK or PARRY?")

    valid_responses = ["duck", "parry"]

    if choice([True, False]):
        if input() == valid_responses[0]:
            print('he tumbles over you')
        elif input() == valid_responses[0]:
            print('he stabs you')

    else:
        if input() == valid_responses[1]:
            print('you trip him up')
        elif input() == valid_responses[1]::
            print('he stabs you')
partdeux()
### END CODE ###

There are some things to note. One is the multiple calls to "input()".
It actually makes sense to do this when you ask the question (you
don't ask someone if they want coffee, take a jog and then get your
answer, do you?).

Thus:
>    print("Do you DUCK or PARRY?")
changes to
>    response = input("Do you DUCK or PARRY?\n")
The "\n" is because "input" doesn't automatically add a new line
whereas print does.

Secondly, you want to catch if it's not valid. You can do this
immediately after calling input (make sure you define valid_responses
first - move it to the top):
>    if response not in valid_responses:
>        # OH NO!

The way I deal with these is often with a while loop until there is a
correct response:

>    while "trying to get response":
>        response = input("Do you DUCK or PARRY?\n")
>        if response in valid_input:
>            break

What this does is constantly ask "Do you DUCK or PARRY" until the
response is valid.


Finally, you want to convert the response to lowercase in case they
thought they had to type in UPPERCASE because it looked like that.
>    while "trying to get response":
>        response = input("Do you DUCK or PARRY?\n").lower()
>        if response in valid_input:
>            break


Done. Here's the code:
(I added three bugs for fun; they're not hard to find but it should
stop you just taking the code without either understanding it or
reading what I wrote. It's also good practice)
### CODE ###
from random import choice

def partdeux():
    valid_responses = ["duck", "parry"]

    print("A man lunges at you with a knife!")

    while "trying to get response":
        response = input("Do you DUCK or PARRY?").lower()

        if response in valid_responses:
            break

    if choice([True, False]):
        if response == valid_responses[0]:
            print("he tumbles over you'')
        elif response == valid_responses[1]:
            print('he stabs you')

    else:
        if response == valid_responses[1]:
            print('you trip him up')
        elif response == valid_responses[1]:
            print('he stabs you')
partdeux()
### END CODE ###

NOTE THAT ROY SMITH'S VERSION IS BETTER, BUT COMPLETELY DIFFERENT CODE