Returning None on Python

Question

My students are making rock, paper, scissors simulations for class. One group has a bug where one of their functions won't return anything. I've checked over it to see if all branches have a return statement, and they do. I've tried a visualizer as well, and the function just stops, and I don't know why.

def Win_Loss(my_history, their_history):
    if len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 's' and my_history[-1] == 'r'):
            return 'p'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 'r' and my_history[-1] == 'p'):
            return 's'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 'p' and my_history[-1] == 's'):
            return 'r'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 's' and my_history[-1] == 'p'):
            return 'r'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 'r' and my_history[-1] == 's'):
            return 'p'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 'p' and my_history[-1] == 'r'):
            return 's'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 's' and my_history[-1] == 's'):
            return 'r'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 'r' and my_history[-1] == 'r'):
            return 'p'

    elif len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 'p' and my_history[-1] == 'p'):
            return 's'
    else:
        return "p"
print(Win_Loss(['s','p','p'],['s','p','p']))

This should be printing 's', but it's printing None.

So, all of the branches definitely do not have a return statement. All the elif branches have a single if statement. If any of those ifs is ever not true, the function will return None. — juanpa.arrivillaga
– juanpa.arrivillaga, Commented Mar 6, 2018 at 20:47
All your conditions are the same: elif len(my_history) > 1 and len(their_history) > 1:.....Try deleting every elif — 001
– 001, Commented Mar 6, 2018 at 20:48
Also, all your outer conditions are the same: len(my_history) > 1 and len(their_history) > 1 so only the first if block will ever execute... — juanpa.arrivillaga
– juanpa.arrivillaga, Commented Mar 6, 2018 at 20:48
Try starting by expressing your intended logic in plain English. — Karl Knechtel
– Karl Knechtel, Commented Mar 6, 2018 at 20:55
Given this question (and your profile), it sounds like you may also find some use for this community: Computer Science Educators. — Ben I.
– Ben I., Commented May 29, 2018 at 18:47

TemporalWolf · Accepted Answer · 2018-03-06 21:27:57Z

3

If any of the inner ifs fail, it will return None, as the outer if/elif was taken and therefore the else: block will not be executed.

To be clear, their entire function is equivalent to:

def Win_Loss(my_history, their_history):
    if len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 's' and my_history[-1] == 'r'):
            return 'p'
    else:
        return "p"

because once that first if is taken, all the other elifs cannot be taken. Since they have the same condition, all of the elifs will never be reached.

If you take the else: out, and just default return "p", it will guarantee a return value:

elif len(my_history) > 1 and len(their_history) > 1:
    if (their_history[-1] == 'p' and my_history[-1] == 'p'):
        return 's'
# If anything else happens
return "p"

but that doesn't fix the root issue.

Alternatively, they could combine the conditionals:

if len(my_history) > 1 and len(their_history) > 1 and
    (their_history[-1] == 's' and my_history[-1] == 'r'):
        return 'p'

which will also avoid the issue, as no internal conditionals can jump the returns and the else: will work as expected.

As others have mentioned, the whole outer if block is superfluous. For my girls (I teach 11-13 y/os), I'd suggest they move the default return to the top... then you don't have to check for each one whether the lists are large enough:

if not (len(my_history) > 1 and len(their_history) > 1):
    return "p"  # default value
elif their_history[-1] == 's' and my_history[-1] == 'r':
    return 'p'
elif their_history[-1] == 'r' and my_history[-1] == 'p':
    return 's'
elif their_history[-1] == 'p' and my_history[-1] == 's':
    return 'r'
elif their_history[-1] == 's' and my_history[-1] == 'p':
    return 'r'
elif their_history[-1] == 'r' and my_history[-1] == 's':
    return 'p'
elif their_history[-1] == 'p' and my_history[-1] == 'r':
    return 's'
elif their_history[-1] == 's' and my_history[-1] == 's':
    return 'r'
elif their_history[-1] == 'r' and my_history[-1] == 'r':
    return 'p'
elif their_history[-1] == 'p' and my_history[-1] == 'p':
    return 's'

and add an exception at the end if we fall out:

raise AssertionError, "a combination not covered was encountered"

which then will make sure we know if we forgot a possibility.

Additional style points which may or may not be worth discussing, depending on their abilities:

By nesting their conditions, they can reduce repetition, although if they are wanting to tweak their bots this is a hindrance rather than a benefit.

if not len(my_history) > 1 and len(their_history) > 1:
    return "p"  # default value

elif their_history[-1] == 's':  # if they threw scissors
    if my_history[-1] == 'r':       # and I threw rock
        return 'p'                      # throw paper
    return 'r'                      # otherwise throw rock

elif their_history[-1] == 'r':
    if my_history[-1] == 'p':
        return 's'
    return 'p'

elif their_history[-1] == 'p': 
    if my_history[-1] == 's':
        return 'r'
    return 's'

edited Mar 6, 2018 at 21:27

answered Mar 6, 2018 at 20:47

TemporalWolf

8,0121 gold badge33 silver badges54 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

glibdud Over a year ago

Niggle: I'm not sure that would do what she expects. She may expect that if the inner if fails, it will fall through to the next elif.

001 Over a year ago

OP just needs to test if len(my_history) > 1 and len(their_history) > 1 once.

Julie Kirkpatrick Over a year ago

That is what I expected. But since it's already gone down one branch, it can't go back. Thanks!

TemporalWolf Over a year ago

@JohnnyMopp I agree that would be the best way, but this is presumably for students, so the minimal changes to get working code will be helpful. I've added a better option along those lines.

jpp · Accepted Answer · 2018-03-07 02:37:55Z

Your elif statements are at the wrong level, as explained by @SiongThyeGoh. The below works fine, albeit inelegantly.

def Win_Loss(my_history, their_history):
    if len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 's' and my_history[-1] == 'r'):
            return 'p'
        elif (their_history[-1] == 'r' and my_history[-1] == 'p'):
            return 's'
        ...

But this would be my favourite solution:

def Win_Loss(my_history, their_history):

    d = {frozenset({'s', 'r'}): 'p',
         frozenset({'r', 'p'}): 's',
         frozenset({'s', 'p'}): 'r',
         frozenset({'s', 's'}): 'r',
         frozenset({'r', 'r'}): 'p',
         frozenset({'p', 'p'}): 's'}

    if len(my_history) > 1 and len(their_history) > 1:
        return d.get(frozenset({their_history[-1], my_history[-1]}))

    else:
        return "p"

print(Win_Loss(['s','p','p'],['s','p','p']))

Siong Thye Goh · Accepted Answer · 2018-03-06 20:53:40Z

0

if len(my_history) > 1 and len(their_history) > 1:
        if (their_history[-1] == 's' and my_history[-1] == 'r'):
            return 'p'

Look at the first three lines, if the first if condition is satisfied but the second is not, then you do not return anything, that is you get None.

Also if this condition passes:

if len(my_history) > 1 and len(their_history) > 1:

then it will get into the first three lines, otherwise, the other elif statement will be ignored since it is the same condition and it will only reaches the else part and returns p.

It can only return either p or None.

edited Mar 6, 2018 at 20:53

answered Mar 6, 2018 at 20:48

Siong Thye Goh

3,58410 gold badges27 silver badges33 bronze badges

Collectives™ on Stack Overflow

Returning None on Python

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

4 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related