exercism / exalysis

Mentoring tool for the Go track on Exercism. Downloads students code, checks it and provides suggestions.
33 stars 13 forks source link

raindrops: re-evaluate advice #78

Open bitfield opened 5 years ago

bitfield commented 5 years ago

Something I've noticed from mentoring approximately 400,000,000 solutions to 'Raindrops' is that a couple of things in Exalysis's output for this exercise can cause confusion.

First, we say:

  • When you use += to append to an empty variable, you could replace it with just = instead. But where you have several almost identical cases, some programmers would prefer to keep the code the same for each case, even if that's not quite optimal: readability beats performance, most of the time.

A lot of students don't understand this. Some take it as the mentor requesting that they should change the first += to an =, so they do. Personally, I think this makes the code less maintainable, because instead of three identical code blocks, we now have two the same and one different, in exchange for a tiny performance improvement. That's the exact opposite of the advice I usually give students, which is to focus on readability first, and only start optimizing when the code has proved to be too slow in production.

The slightly deeper problem with this is that (in a very early core exercise) it subconsciously trains the students that what we're really looking for is absolute performance. The message is that they should tweak and twiddle every line of code, benchmarking it to death until they've wrung out the last nanosecond. This leads to some over-engineered and overcomplicated solutions all the way down the track.

Secondly, if the student is using a strings.Builder or a bytes.Buffer, we say:

  • I see you are using a %s here. That works, but it's probably overkill for this exercise. The string append operator += is faster and uses less memory. You can run the benchmarks as described in the exercise instructions, and check that for yourself.

I get a lot of puzzled students asking why this is, and which is better to use in the general case, strings.Builder or +=. Obviously strings.Builder is what you should use 99% of the time, and it's only not the fastest option here because we have an unrealistically small number of appends. I think sending students down this benchmarking rabbit-hole creates a lot of confusion, and effectively penalises them for their superior knowledge of the standard library. Additionally, as before, it sends the message that everything in Go programming is about performance optimization.

I suggest that we drop the advice about changing += to =, and don't mention strings.Builder either. If they use it, great. If not, that's fine too. This is such an early exercise I think it's important not to overload students with too many concepts.

tehsphinx commented 5 years ago

I fully agree on the first one (+=). I was about to propose to remove it as well.

About string building:

All in all I don't want to punish the student in any way. I would like to encourage him to think about the choices he has. And just because there is a strings.Builder it does not mean it is the best option all the time.

dysolution commented 5 years ago

Is there a later exercise where the performance concerns surrounding bytes.Buffer or strings.Builder might be more important? I'd rather we didn't overwhelm people early in the track with too many performance-related suggestions.

If in practice it seems to be confusing or frustrating students at this stage of the track, I would think between the competing concerns of imparting useful information at the first possible chance they could encounter something relevant in the track, vs. keeping them encouraged and moving toward more complex exercises, we should be optimizing for the latter.

bitfield commented 5 years ago

Yes, this is the point. I agree entirely with everything @tehsphinx says; my only concern is that I think this exercise (which is No. 3 on the track) is a little too early to introduce this issue. I wouldn't say that unless I'd seen a significant number of students being confused or sidetracked by it.

When I'm mentoring exercises, the level and nature of the feedback I give students varies a lot: for early exercises, I'm concentrating mostly on encouraging them, building good workflow habits like automatic lint and format, and emphasising simple, readable code. For later exercises I focus more on things like performance vs readability trade-offs, subtle improvements and optimizations, understanding exactly when is the right time to use a certain library or construct, and when not to.

It's just like being a consultant, which is my day job. That involves introducing people to a lot of new information and concepts, and you have to do it step-by-step and not overwhelm them. In particular, at the beginning people are usually just looking to be told what to do, and given something that works well in most situations. Once they become familiar with that pattern, they'll be ready to start learning alternatives, along with a more conceptual understanding of which alternatives are better in which situations.

What you definitely don't do is start by saying "Well, you could do this, which has pros and cons X, Y, and Z, or you could do this, which has pros and cons A, B, and C, but sometimes it's better to do that..." Faced with a paralysis of choice, they get completely confused and demotivated and feel like giving up, which is not what we want.

tehsphinx commented 5 years ago

Before we make a decision here I would like to throw in one more thought:

There are many students where the concerns you raised are very much valid. Go might be their first programming language or they have already looked at other languages but are not fluent in either of them.

But then there are also many students that are already fluent in at least one other programming language and that just want to get to know how strings are best concatenated in Go. They might not explicitly ask but have probably wondered already and maybe even realized that there are multiple ways to do this.

Now my suggestion:

Something along the lines:

Did you know there are multiple ways you can build a string in Go? There is strings.Builder which is good for building complex strings by appending again and again. There is bytes.Buffer working on byte level but which can also be used to build strings. Then again you can simply append strings with the + operator chaining them together as you like. You could also append all the parts to a slice and use strings.Join to put the string together afterwards. Also there is fmt.Sprintf which you probably used in the twofer exercise but which does not really fit here. If you want to get to know your options have a closer look a the strings, bytes and fmt packages.

Feel free to make it proper fluent English ;)

bitfield commented 5 years ago

I wish Exalysis could detect from the student's code how advanced of a programmer they are, and tailor its advice accordingly! Sadly, that probably requires AGI.

tehsphinx commented 5 years ago

To myself: Maybe add this reference: https://syslog.ravelin.com/bytes-buffer-i-thought-you-were-my-friend-4148fd001229