comparative code examples in tutorial/guides should be minimal diffs

[pnkfelix]

Sometimes concepts are explained by presenting a piece of code, then explaining some problem with it, and finally showing one or more revised versions that fix the problem in various ways.

The problem is that if you make many changes in the revision, it can obscure the point that is being made.

One particular example (though there may be others; the above is just a general principle):

In the sample code from benchmarks and the optimizer, we first see:

    bh.iter(|| {
            range(0, 1000).fold(0, |old, new| old ^ new);
        });

which has a problem, and then we see as the fix:

    bh.iter(|| range(0, 1000).fold(0, |old, new| old ^ new))

There are three changes between these two code fragments:

1. The semi-colon from the statement within the closure's body was removed
2. The closure's body was simplified from { expr } to expr (and it was all folded onto one line in the process).
3. The iter call itself now has no semi-colon terminating it.

So, first the question: Were both changes (1.) and (3.) necessary? Or just (1.)? One cannot tell from the presentation.

Second, the suggestion: while I like the second code fragment as code on its own, I think achieving the effect by removing semi-colons is too subtle for this tutorial's presentation, especially when mixed with other (semantically insignificant) changes to the fragment. I basically am suggesting that the revised code should look just like the original, except with one very apparent change, like so:

    bh.iter(|| {
            return range(0, 1000).fold(0, |old, new| old ^ new);
        });

return statements did not used to be legal within closures, but I believe they are now, so this should work. (if it is considered poor style to use return within closures, then I would settle for just dropping the semi-colon, but with a comment added pointing out where it has been removed.)

Anyway, this is a lot of description text for a small change to one tutorial example. My goal is not to just get one example fixed; its to encourage the above as a style of exposition in our tutorials.

[pnkfelix]

part of #11755.

[nikomatsakis]

On Thu, Feb 13, 2014 at 04:56:25AM -0800, Felix S Klock II wrote:

    bh.iter(|| {
            return range(0, 1000).fold(0, |old, new| old ^ new);
        });

Without context, what about:

   bh.iter(|| {
           range(0, 1000).fold(0, |old, new| old ^ new) // <-- note lack of semicolon
       });

[pnkfelix]

I would say that qualifies as:

(if it is considered poor style to use return within closures, then I would settle for just dropping the semi-colon, but with a comment added pointing out where it has been removed.)

[huonw]

I updated the example in #15039 to include a comment pointing out the lack of semicolon (and reducing the number of irrelevant changes).

[pnkfelix]

The particular problem I was pointing out in this ticket has been fixed.

I still believe that the more general philosophy being described in this ticket should be applied to all of our documentation code examples. But that is not exactly an actionable issue, apart from doing a full survey of the docs to identify non-minimal diffs.

I'm closing this and cc'ing @steveklabnik just he is aware of the general problem that one can sometimes encounter.

[steveklabnik]

Yup, I agree, and have already made one or two comments making sure we do this in examples.