Skip to content

Task 1 of Meltdown Mitigation is misleading #2535

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
punkstarman opened this issue Sep 3, 2021 · 7 comments
Closed

Task 1 of Meltdown Mitigation is misleading #2535

punkstarman opened this issue Sep 3, 2021 · 7 comments
Assignees

Comments

@punkstarman
Copy link

One of the most common blunders a novice can make is to use if-elif-else blocks instead of using a single return statement.

For instance

if a:
    return True
elif b:
    return True
else:
    return False

should simply be written

return a or b

The introduction sets up using if:elif:else: blocks. I fear that a novice will blindly fall into the trap of solving task 1 using an if.

def is_criticality_balanced(temperature, neutrons_emitted):
    if temperature < 800 and neutrons_emitted > 500 and temperature * neutrons_emitted < 500_000:
        return True
    else:
        return False

or something even worse, when it should be solved with a single return statement.

def is_criticality_balanced(temperature, neutrons_emitted):
    return temperature < 800 and neutrons_emitted > 500 and temperature * neutrons_emitted < 500_000
@BethanyG
Copy link
Member

BethanyG commented Sep 6, 2021

@punkstarman 👋🏽 Thanks for logging this.

Concept Exercises are quite different from the "regular" practice exercises that were carried over from V2. They're intended to teach a specific concept, and must only use the concepts introduced as part of their dependency tree. Since this concept exercise is designed to teach conditionals, we can't (even though bools is covered first) have the exemplar.py showing the return of a bool, and not showing if, elif, and else in action.

I am happy to have someone propose revisions / additions to this exercise provided that it thoroughly covers the use of if, elif, else and only use the techniques taught in basics (Guidos Gorgeous Lasagna) and bools (Ghost Gobble Arcade Game). But I do think it's (with the changes below) decent as it stands.

Having said that - agree that multiple returns here is not ideal -- especially when it pisses off PyLint. To that end, I've re-written the exemplar.py file in PR #2556 to use a variable for return instead.

@BethanyG
Copy link
Member

BethanyG commented Sep 6, 2021

I've merged #2556. Please let me know if you have further questions/issues/comments. Thanks!

@punkstarman
Copy link
Author

Having said that - agree that multiple returns here is not ideal -- especially when it pisses off PyLint. To that end, I've re-written the exemplar.py file in PR #2556 to use a variable for return instead.

PyLint is wrong in this instance. Multiple return statements is the right thing to do.

I've merged #2556.
IMO, this has made things worse. I will suggest something different.

@BethanyG
Copy link
Member

BethanyG commented Sep 7, 2021

Suggest away! PRs on this welcome, with the caveats listed above. 😄 I will hold this issue open, and flag it as a discussion.

@kbuc
Copy link
Contributor

kbuc commented Sep 9, 2021

if a:
    return True
elif b:
    return True
else:
    return False

return a or b

I agree with this as long as the items are bools.
What if a is an integer, or list, or something that has an implicit boolean meaning? 5 or False returns 5, not True, but the above mentioned if structure does return True. While one can achieve the same with bool(5) or False, it might be a bit confusing for a beginner.

I would not simply discard the good old if. But yes, there is definitely space for a lesson with an advanced boolean handling (including the short-circuits, implicit meanings, one line returns etc).

@punkstarman
Copy link
Author

I agree with this as long as the items are bools.

Absolutely

I would not simply discard the good old if.

I do not mean to discard it. I mean to make proper use of it in context.

@BethanyG
Copy link
Member

I am going to close this issue. The tasks in this exercise are being reviewed/revised, but we are unlikely to change Task 1 to not use an if. We may revisit this in the future, but we need to keep this exercise focused on the use of conditionals (not Booleans) for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants