marijnh / Eloquent-JavaScript

The sources for the Eloquent JavaScript book
https://eloquentjavascript.net
3.01k stars 795 forks source link

The description of reduce is misleading #387

Closed johanrex closed 6 years ago

johanrex commented 6 years ago

In chapter 5 Higher Order Functions one can read:


function reduce(array, combine, start) {
  let current = start;
  for (let element of array) {
    current = combine(current, element);
  }
  return current;
}

Furthermore:

If your array contains at least one element, you are allowed to leave off the start argument. The method will take the first element of the array as its start value and start reducing at the second element.

And the example given for this:

console.log([1, 2, 3, 4].reduce((a, b) => a + b));
// → 10

The above information is not sufficient and actually misleading to describe the next example:


function characterCount(script) {
  return script.ranges.reduce((count, [from, to]) => {
    return count + (to - from);
  }, 0);
}

Using the above description is very confusing when reducing an array of arrays as in the characterCount example. One is led to believe that "count" is assigned the first element in the array "ranges", as per the sample implementation. This works for numbers but not arrays. It does not make sense and does not lead to a reduce at all.

The MDN explanation (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/reduce) makes it very clear what is going on though:

The reduce() method applies a function against an accumulator and each element in the array (from left to right) to reduce it to a single value.

I hope I could convey the confusion a first time reader is faced with. The example implementation does not prepare you to understand reduce on array of arrays since it actually assigns the first element to the accumulator. If that element is an array it all falls apart.

marijnh commented 6 years ago

The characterCount example does not omit the second argument, so the situation described in the paragraph you first quoted simply doesn't apply there.

johanrex commented 6 years ago

Correct. The characterCount example does not omit the second argument. That is not the issue.

The example reduce code does "let current = start;". I.e. current item is also used as an accumulator. That situation does not prepare you for understanding how reduce on array of arrays work. With the knowledge from the reduce example implementation it's not possible to understand this expression:

 return script.ranges.reduce((count, [from, to]) => {
    return count + (to - from);
  }, 0);

I.e nowhere is it mentioned that there is an accumulator binding that is separate from the array item.

marijnh commented 6 years ago

That situation does not prepare you for understanding how reduce on array of arrays work.

The example implementation works on arrays. It passes current to the reducer function as first argument, so yes, it does show the accumulator binding.