Skip to content
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

Add mention of swap idiom to 'The Old Switcheroo' exercise #917

Merged
merged 8 commits into from
Apr 6, 2021

Conversation

svaberg
Copy link
Contributor

@svaberg svaberg commented Mar 16, 2021

This pull request proposes to close #883 by adding a brief note about the switch idiom a, b = b, a in The Old Switcheroo exercise and linking to the description of the switch idiom already present in the Extra Exercises page.

In the Extra Exercises page it also replaces a, b = [b, a] with the more idiomatic and simpler a, b = b, a.

Copy link
Contributor

@ldko ldko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @svaberg ,
Thanks very much for submitting this contribution to the lesson. In looking back at the history of the topic of swapping variables in this lesson, I see #460 ultimately decided not to bring in the swapping variable idiom of a, b = b, a (at least not in the list episode). I think the reasoning given there still makes sense in the context of the functions lesson, but let's see what @maxim-belkin has to say as well.

_extras/extra_exercises.md Show resolved Hide resolved
> >
> > Note that the recommended way of swapping the content of `a` and `b` in Python is
> > `a, b = b, a`. You can see why this works in the
> > [Additional Exercises]({{ page.root }}/extra_exercises/index.html).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the resolution of the issue #460, I am not sure we want to include this idiom. Also, with the challenge ultimately being about variable scope rather than how to swap variable values, it may be distracting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about this overnight. From a distractions perspective the whole swapping functionality of the function may be distracting from the challenge itself. Maybe the The Old Switcheroo exercise should be replaced by a function that does not switch variables at all.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you about the swapping being distracting; this seems something of a trick question. Looking at the other exercises based on your comment, I notice the challenge titled "Variables Inside and Outside Functions" seems to highlight the same concept of variable scope. Perhaps the Old Switcheroo could go away altogether in that case. Additionally, scanning over the episode material, I am not seeing us actually introduce the concept of variable scope or local variable...if that is the case, perhaps it is something we need to add... @maxim-belkin ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the Variables Inside and Outside Functions challenge teaches essentially the same content, and in a less cluttered way. As such, maybe The Old Switcheroo should indeed be removed.

Maybe some of the explanatory text from the The Old Switcheroo solution about local variables could be shifted over to the Variables Inside and Outside Functions?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to move some of the description, such as adding to the end of what is there:

When the f2k function is called, it creates a local variable k. The function does not return any values and does not alter k outside of its local copy. Therefore the original value of k remains unchanged.

I think it could also be useful to create a glossary entry for local variable and link to it there. The text could also differ somewhat if you think it could be explained better in a different way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Does this mean we are moving towards removing The Old Switcheroo? I'll try and implement the changes you propose.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless @maxim-belkin or others in the community speaks up about why we should keep the exercise, it would be great for you to carry out the proposed changes. Thank you, @svaberg!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've carried out all the proposed changes but I have not yet removed the Old Switcheroo. I'll do that now in a separate commit.

@maxim-belkin
Copy link
Contributor

I usually cover this "Old switcheroo" exercise when I teach our lesson, so I'd like to propose to modify it instead of removing it. We can do something as primitive as multiplying a by 10 and adding 10 to b instead of switching their values. I think it's an important demonstration for those who're new to functions.

@ldko
Copy link
Contributor

ldko commented Mar 30, 2021

Hi @maxim-belkin, can you clarify what you mean as being "an important demonstration for those who're new to functions"? Are you referring to something not demonstrated in the "Variables Inside and Outside Functions" that @svaberg expanded upon in this PR?

@maxim-belkin
Copy link
Contributor

I'm referring to the same idea/concept as in the f2k exercise but I think it's an important one so would prefer our lesson to have more than a single exercise devoted to it. If you think we can drop it -- so be it, but it's worth noting that other SWC/DC Python lessons don't talk about it much.

@ldko
Copy link
Contributor

ldko commented Mar 30, 2021

@maxim-belkin I agree that the concept related to variable scope is important. Because of that I would prefer to see it mentioned in the main content of the episode, perhaps in the Composing Functions section. There are 10 challenges in this episode, which can make it difficult for an instructor trying to pick what can be included in the time frame of a workshop. If one or both of the challenges referred to here are skipped, then the learners will miss the concept or its emphasis.

What do you think about adding something like this after the fahr_to_kelvin example:

Note, variables created inside of a function, referred to as local variables, no longer exist once the function is done executing.
print('Again, Kelvin was:', temp_k)
Traceback (most recent call last):
File "", line 1, in
NameError: name 'temp_k' is not defined
If you want to reuse the temperature in Kelvin after you have calculated it with fahr_to_kelvin, you can store the result of the function call in a variable.
temp_kelvin = fahr_to_kelvin(212.0)
print('Kelvin was:', temp_kelvin)
Kelvin was: 373.15

I think it would be faster and more effective for an instructor to cover this here and have learners do one challenge than to have learners do both challenges.

Copy link
Contributor

@ldko ldko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing all my comments, @svaberg . I have marked a couple very minor things.

_episodes/08-func.md Outdated Show resolved Hide resolved
reference.md Outdated Show resolved Hide resolved
@maxim-belkin
Copy link
Contributor

What do you think about adding something like this after the fahr_to_kelvin example:

I think this is a great suggestion. In general, it might be worth converting existing material to a (small) section devoted to variable scopes.

@ldko
Copy link
Contributor

ldko commented Mar 30, 2021

Ok, @maxim-belkin I can open a separate PR to add a section about variable scope.

svaberg and others added 2 commits March 31, 2021 18:08
Add backticks around mention of f2k function.

Co-authored-by: Lauren Ko <lauren.ko@unt.edu>
Remove extraneous blank space.

Co-authored-by: Lauren Ko <lauren.ko@unt.edu>
@ldko ldko requested a review from maxim-belkin April 6, 2021 14:43
Copy link
Contributor

@maxim-belkin maxim-belkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good! Thank you for your contribution, Dag!

@maxim-belkin maxim-belkin merged commit 292c57a into swcarpentry:gh-pages Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lesson Contribution - Swapping variables
3 participants