Add "groupBy" method

[katanacrimson]

Proposed method:

Similar to sortBy, but instead of just sorting by the returned value, it will rebuild the list using the returned value as the entry's index for an array of entries. This would be useful when trying to group or categorize.

e.g.

  { pid: '2384',
    name: 'node run',
    exe: 'node.exe',
    path: 'C:\\Program Files\\nodejs\\node.exe',
    cmdline: 'node.exe',
    memuse: '13348864' },
  { pid: '5000',
    name: 'C:\\Windows\\system32\\wbem\\wmiprvse.exe',
    exe: 'wmiprvse.exe',
    path: 'C:\\Windows\\system32\\wbem\\wmiprvse.exe',
    cmdline: 'WmiPrvSE.exe',
    memuse: '6266880' } ]

with a groupBy method, you'd be able to group individual processes by 'exe' - something that nothing else (that I can see) would allow for.

[durango]

👍 This would affectively eliminate lodash for me :)

[brianmaissy]

If I understand correctly, your proposal would work as follows?

var ungrouped = [
  {name: Alice, age: 12},
  {name: Bob, age: 12},
  {name: Carol, age: 13}
]

function getAge(person, cb){
  cb(null, person.age);
}

async.groupBy(ungrouped, getAge, function(err, results){
  // here, results is {
  //   12: [ {name: Alice, age: 12}, {name: Bob, age: 12} ],
  //   13: [ {name: Carol, age: 13} ]
  // }
});

If so, I would propose implementing it by delegating to async.reduce

[katanacrimson]

@brianmaissy Spot on - that would be the intention of such a method, yes. I do wonder what the performance costs would be by using reduce for it, though...

[brianmaissy]

You're right, I take that suggestion back. I would implement it similarly to the implementation of sortBy: first by using async.map to calculate the 'grouping criteria' and then manually building the grouped result object with one additional pass through the results.

[caolan]

The API and implementation using async.map sound good (since async.reduce is always in series). I do, however, wonder whether this sort of function should be included in async - I imagine it's not particularly common and an async.map followed by some underscore function would do the same thing. Same goes for async.sortBy... how much should the library wander into general array utilities? - discussion on that point welcome

[danmilon]

Same feelings. This is not about control flow, which async is for. While it is useful and all, it just doesn't fit here.

[brianmaissy]

Hmm, in retrospect that is a good point. "async.map followed by some underscore function" is basically what its implementation consists of, minus underscore. I figured that if sortBy fits into async's mission, then so would groupBy, even if it is less common. But now that you question sortBy, I think you have a good point, this basically is starting on the process of rebuilding an asynchronous underscore, which is not a great idea.

[brianmaissy]

How did sortBy originally get in? Of course now that it's there, it shouldn't be removed, but maybe we should add something to the documentation saying that this isn't really the goal of async

[caolan]

@brianmaissy when creating a new library it sometimes takes a while to find it's appropriate boundaries. I think sortBy shouldn't have been included, and I'd even be quite tempted to deprecate it instead of leaving it hanging around. I don't think there will be many people using it and it's easy to implement yourself. Of course, we'll have to wait for a major-version to introduce API changes like this.

[brianmaissy]

Even if it is not deprecated now, should we put something in the documentation letting contributors know not to use it as a model for other functions?

[dyllan-to-you]

Discovering this ticket 6 years later, it looks like the feature creep has been officially enshrined