Fix meltdown mitigation task 1 #2566
Conversation
| if 80 <= percentage_range <= 100: | ||
| efficiency_level = 'green' | ||
| return 'green' | ||
| elif 60 <= percentage_range <= 79: | ||
| efficiency_level = 'orange' | ||
| return 'orange' | ||
| elif 30 <= percentage_range <= 59: | ||
| efficiency_level = 'red' | ||
| return 'red' | ||
| else: | ||
| efficiency_level = 'black' | ||
|
|
||
| return efficiency_level | ||
| return 'black' |
There was a problem hiding this comment.
This task is getting revised via #2565, so I think we should back out these changes.
| Bear in mind that in the special case where a function returns a boolean, it should not use an `if`. | ||
| Rather it should return the condition directly. | ||
|
|
||
| ```python | ||
| # DO NOT use `if` and return `True` | ||
| >>> def is_leap_year(year): | ||
| if (year % 4 == 0 and year % 100 != 0) or year % 400 == 0: | ||
| return True | ||
| else: | ||
| return False | ||
|
|
||
| # instead return the condition directly | ||
| >>> def is_leap_year(year): | ||
| return (year % 4 == 0 and year % 100 != 0) or year % 400 == 0 | ||
| ``` | ||
|
|
There was a problem hiding this comment.
While this is excellent information to discuss, I don't think this exercise (which is focused on conditionals) is the place to do it.
This note sets an exception/rule about Booleans without going into the nuance that one can return an expression directly without doing a check and assigning to an intermediary state variable -- which isn't just about conditionals and Booleans. The whole topic is better left to a mentor discussion/notes or explored in detail in an exercise on structuring functions/handling returns.
So this needs to be backed out of the exercise introduction. However, it would be good information to include in the about.md document (for display after the student completes the exercise), if you would like to PR it there.
There was a problem hiding this comment.
I strongly disagree, unless someone can find a way to fix Task 1 so that it is not misleading (see another discussion).
The learning exercise on Booleans (Ghost Gobble Arcade Game) already has students returning expressions directly, and this concept is explained in the Basics exercise Guido's Gorgeous Lasagna.
Also, knowing when not to use a tool is part of mastering that tool.
| print("z is greater than x and y") | ||
| ... | ||
| >>> z is great than x and y | ||
| >>> z is greater than x and y |
| balanced = True | ||
|
|
||
| return balanced | ||
| return temperature < 800 and neutrons_emitted > 500 and output < 500000 |
There was a problem hiding this comment.
As stated in the issue, this exercise is about conditionals, and every task should use or practice them - as contrived as that feels. This task in the exemplar now displays only the use of Booleans. If this task cannot be re-written to use/display the use of conditionals, then another task should be made or this task should be omitted from the exercise.
There was a problem hiding this comment.
As it is, the task is misleading, so
another task should be made or this task should be omitted from the exercise.
but I do not have the inspiration for this at the moment.
There was a problem hiding this comment.
In addition to the in-line comments (which did not trigger "request change" for some reason), as @mukeshgurpude has stated, the CI / tests are failing due to an error on line 63.
|
Closing since we cannot come to agreement on task 1, and because there have been changes to this exercise via PR2568 that have made substantial changes. |
fixes #2535