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

06 - composing functions - refactor to build up a more complicated function #366

Open
dstndstn opened this Issue Mar 28, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@dstndstn
Contributor

dstndstn commented Mar 28, 2017

Currently, the fahr_to_celsius function is composed of fahr_to_kelvin and kelvin_to_celsius, both of which involve the "magic number" 273.15 -- this seems like bad form; and fahr_to_kelvin includes both conversion functions, so the composition doesn't really make sense, since it "undoes" a conversion that was already done.

@tbekolay

This comment has been minimized.

Contributor

tbekolay commented Mar 29, 2017

You're right that the composed fahr_to_celsius isn't the most efficient way to do the conversion (both computationally and conceptually). But the point of the topic isn't to present efficient code, it's to introduce the concept of function composition. It's relatively easy to motivate composition when presenting this lesson by appealing to laziness -- "Who remembers the formula to convert from fahrenheit to celsius? How confident are you in that formula? Rather than looking it up to be sure, why don't we use the functions we've already made to save us the time of looking it up." This follows from the general idea that your time as a programmer is more valuable than wasting a few CPU cycles on a function call that could be avoided.

Currently we don't talk about "magic numbers" in the lesson. IMO, this is a bit of a premature optimization when instructing novice programmers -- yes, as you scale up, using variables for these numbers will make your code easier to read and easier to modify. But is that a necessary anxiety to give a novice programmer when they're just getting their feet wet? Further, 273.15 isn't an egregious example of a magic number as it will never need to be changed to something else. On the other hand, there is some precedent about talking about code style in callouts in the "What's in a name" callout in episode 2. I wouldn't be opposed to a callout in episode 6 about magic numbers, but I don't think I would change the functions that are there already.

@dstndstn

This comment has been minimized.

Contributor

dstndstn commented Mar 29, 2017

I'm concerned about the conceptual complexity, not the computational. The way the functions are currently laid out, it's like 2-steps-forward-1-step-back to do fahr to celsius via fahr to celsius to kelvin plus kelvin to celsius. Sure, I can see the "laziness" appeal, but I don't see this as a good example of building up complexity.

Let me put together a quick PR just to see what it looks like. I think it'll be cleaner, but I've never taught this lesson so I could be wrong!

rgaiacs pushed a commit to rgaiacs/swc-python-novice-inflammation that referenced this issue May 6, 2017

Merge pull request swcarpentry#366 from gvwilson/updating-amy
Filling in steps for trainers on AMY
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment