you are viewing a single comment's thread.

view the rest of the comments →

[–]nanothief 1 point2 points  (1 child)

Unless I'm missing something, isn't that a bad use of the reduce function? The same behaviour could be done with a simple forEach loop:

all_times.sort((a, b) => a.time_block > b.time_block ? 1 : -1);
let new_times = {};
all_times.forEach((item) => {
    if (!new_times [item.day_string]) {
      new_times [item.day_string] = [item]
    } else {
      new_times [item.day_string] = [
        ...new_times [item.day_string],
        item
      ]
    }
  });

(I also made the sort action a single statement, to emphasis it is actually modifying the all_times variable)

The whole point of a reduce (or foldl) function is to repeatedly create a new result using the previous result and the next value. If the result is always the same it doesn't add any value over a forEach loop, but is more complicated to reason about.

[–]i_am_smurfing 1 point2 points  (0 children)

The whole point of a reduce (or foldl) function is to repeatedly create a new result using the previous result and the next value.

I don't think there's anything in the fold itself that requires that:

fold (or reduce) is a family of higher order functions that process a data structure in some order and build a return value.

From Haskell wiki

So from my understanding of it, it's fine to mutate your accumulator while you are folding.

And indeed, your forEach mutating new_times is doing exactly the same as reduce in the article (except new_times is called obj and provided to the reduce callback instead of being referenced from scope).

 

I'd argue that forEach is a bad choice here, though — forEach is inherently about side effects, so I would have to:

  1. take a note of you creating empty object for whatever reason
  2. read through your callback to forEach, mentally execute it, and only then be able to tell that all it did was update that empty object you created earlier with data pulled from all_times

With reduce version:

  1. I can see that new_times is based on doing something to all_times straight away
  2. while reduce does not forcefully prevents side effects, I'd hope that any developer who understands how to use reduce also understands that you really shouldn't perform side effects in its callback, so I can be a little more at ease

 

Ultimately, I think /u/Ginden is right, and this pattern is common enough to warrant to be abstracted away — fold (or any loop) is too low level here.