Functions concept

[bobtfish]

This is my first attempt at something as ambitious as a full exercise and a concept, so any/all feedback and tips very gratefully received.

Specific things that I'm unsure about / have questions on:

• Fitting this concept/exercise into the existing hierarchy - I've done this somewhat, but if I add dependencies for everything that's relevant then I create cycles. Feedback welcomes from someone who has a more holistic view of the progression of concepts they think is right for the Go track.
• The concept introduction and exercise introduction are nearly identical. Is this OK/appropriate? (Should I just be using .docs/introduction.md.tpl if so?)
• .meta/design.md - this is my first attempt at one of these, I've read a few others but it's largely cargo cult.
• Should my exercise include forked_from? It's not clear to me from the docs what the contents of this field would be? (I.e. is it just same track exercises, or can you reference cross tracks etc)

Fixes #1642

[ErikSchierboom]

Thank you so much for doing this! I've roughly reviewed this, but decided to hold off on some reviewing more to first solve the dependency cycles.

Fitting this concept/exercise into the existing hierarchy - I've done this somewhat, but if I add dependencies for everything that's relevant then I create cycles. Feedback welcomes from someone who has a more holistic view of the progression of concepts they think is right for the Go track.

@junedev could you perhaps take a look at how you envision the concept hierarchy? One immediate issue I could see was that this exercise depends on slices, but that the exercise teaching slices also teaches multiple returns. We could consider moving the multiple returns to this exercise? It's also important to note that we should decide if we want this to be an early exercise or not (as in: which exercises should depend on this)?

The concept introduction and exercise introduction are nearly identical. Is this OK/appropriate? (Should I just be using .docs/introduction.md.tpl if so?)

This is perfectly ok. At some point the .tpl files will get good support, but we have only basic support for it now so I'd ignore it.

.meta/design.md - this is my first attempt at one of these, I've read a few others but it's largely cargo cult.

It's perfectly fine.

Should my exercise include forked_from? It's not clear to me from the docs what the contents of this field would be? (I.e. is it just same track exercises, or can you reference cross tracks etc)

Forked from is exclusively for cross-track references. So in this case it should be:

`"forked_from": ["javascript/lasagna-master"]

[bobtfish]

Thank you so much for doing this! I've roughly reviewed this, but decided to hold off on some reviewing more to first solve the dependency cycles.

Sounds good - thanks for the rough cut that was really helpful.

Agree that we should sort the high level dependency issues and decide where this fits and what it can/can't depend on before we get into the minutiae.

[junedev]

Disclaimer: I did not read the PR content yet, I am mostly responding to the question regarding the concept tree.

The functions concept should include explaining multiple return values, not as a separate concept just as part of the content. This is where it belongs imo. Content can probably be copied. Thus multiple return values can be safely used as part of the exercise here.

This exercise should have slices and structs as prerequisites. When you modify the scaleRecipe exercise to use a slice of structs instead of a map that should be enough to make it work. (Make sure not to use range, it not available yet. Just use a normal for loop.)

We can then later go ahead and remove the multiple return values concept from slices. The exercise would just need a minimal change, there is no conceptual dependency (unlike for maps, there you need multiple return values). Until then I don't think anyone will think this is bad track because we repeat a couple of lines of concept content. Everything that depended on multiple return values will then depend on functions later.

The general hierarchy on the end would be that the student learns the basic types and conditionals, that unlocks slices and structs, which unlocks functions which unlocks maps.

With regards to pointers I would go with the "learn the details later approach". Strictly speaking you would need pointers to understand how the complex types are passed to the function. However currently pointers has maps as prerequisite and I would like to keep it a bit further down the tree. So instead of going into the details here I would just talk about how the passing works for "reference types" and primitives.

Does that help/ make sense?

[junedev]

I had a quick look at the exemplar file:

• I dont think the first task is needed here, it was about "not passing an argument" which does not exist in Go
• The second task should be changed to practice Gos version of a default parameter (an if statement). If the second argument is 0, use 2 as default.
  That Go has no default params should be mentioned in the introduction.

[ErikSchierboom]

@bobtfish Does @junedev's explanation help sort the dependencies issue?

[bobtfish]

@bobtfish Does @junedev's explanation help sort the dependencies issue?

Yes! Thanks @junedev - I'll work on getting this into shape in the next day or two and let you know when it's ready for review.

[ErikSchierboom]

@junedev I've opened an issue: #1728

[junedev]

@bobtfish Another thought regarding scaleRecipe. Instead of making it a slice of structs you could also just have a slice of floats an say "each item represents the amount of some ingredient ...". The task is about not modifying the argument so a slice will do to make this point.

[junedev]

@bobtfish Setting this to draft for now. Mark as "ready for review" again whenever you are ready.

[bobtfish]

Make sure not to use range, it not available yet. Just use a normal for loop.

I'm unsure about this - it's totally possible to do this, but the exercise is a lot nicer to solve if you can use range, and there isn't anything in the dependencies at the moment which stops this from depending on range-iteration.

Why do we imagine that this has to be before we cover range-iteration?

[junedev]

Why do we imagine that this has to be before we cover range-iteration?

Range iteration need to show how to iterate over maps so maps need to be known to the student, maps need multiple return values from here so we can't move maps before functions.

[bobtfish]

Why do we imagine that this has to be before we cover range-iteration?

Range iteration need to show how to iterate over maps so maps need to be known to the student, maps need multiple return values from here so we can't move maps before functions.

Aha, got it, thanks for the explanation. I'll take range-iteration out later tonight.

[junedev]

Well done overall! I really like the examples you used in the concepts.

Some more small things need to be fixed before this can be merged and the merge conflict needs to be resolved.

[bobtfish]

Well done overall! I really like the examples you used in the concepts.

Thanks!

Some more small things need to be fixed before this can be merged and the merge conflict needs to be resolved.

All fixed up. I wasn't 100% sure what you were suggesting around the PrintHelloName example, but I have had a go at amending it.

I've also rebased and squashed as it was getting unwieldy - no point committing adding then removing then re-adding the same thing , and that also allowed me to fix the merge conflict so it'll merge cleanly without a load of back-and-forth merges to make future history exploration easy. (I'm not sure if there's a house style/recommendation about merges/rebases for PRs in progress)