Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IterableMixin<E>#groupBy #11159

Closed
DartBot opened this issue Jun 8, 2013 · 14 comments

Comments

@DartBot
Copy link

commented Jun 8, 2013

This issue was originally filed by @MarkBennett


What steps will reproduce the problem?

  1. Create a Iterable or Hash
  2. Attempt to collect like instances or keys
  3. Fail

What is the expected output? What do you see instead?

A groupBy function should exist which transforms an iterable in to a HashMap where the keys are the common value. It's possible a mapping function could be used to transform the original values before they are grouped.

What version of the product are you using? On what operating system?

Mac OS, Dart SDK 23552

Please provide any additional information below.

This came up during a weekend CodeRetreat when I was pairing with a C# dev, then again when reviewing https://github.com/dartist/101LinqSamples

@kasperl

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2013

Added Area-Library, Triaged labels.

@lrhn

This comment has been minimized.

Copy link
Member

commented Jun 10, 2013

So you are asking for:
 Iterable<T>.groupBy(Object f(T element)) -> Map<Object,Iterable<T>>
so that if f(a) == f(b) and map = [a,b,c].groupBy(f) then the map[f(a)] contains a and b?


Removed Type-Defect label.
Added Type-Enhancement label.

@floitschG

This comment has been minimized.

Copy link
Contributor

commented Jun 10, 2013

Sounds ok.
It's not a high priority, though.
Also would need to be implemented for Streams.


Added this to the Later milestone.
Added Ready-to-implement, Accepted labels.

@DartBot

This comment has been minimized.

Copy link
Author

commented Jun 12, 2013

This comment was originally written by demis.b...@gmail.com


A more functional 'order', 'group', 'join', 'joinGroup', 'range'
are just some of the utilities I had to add in order to port the C# 101 LINQ samples across.

https://github.com/dartist/101LinqSamples

I found it's really limiting only being able to use the functional utils that's shipped in the core libs. Most languages (JS, Ruby, C#, Scala, Clojure, Go, etc) allows us to extend or at least 'visibly extend' built-in types without our own custom methods.

Dart's lack of 'extension methods' is really behind other languages in this regard, which visibly affects the surface area of the API impacting readability since we're unable to idiomatically chain transformation methods.

@kasperl

This comment has been minimized.

Copy link
Contributor

commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@lrhn

This comment has been minimized.

Copy link
Member

commented Oct 1, 2014

Adding new members to Iterable at this point is not a good idea.
There are many existing implementations of Iterable that would need updating.


Removed Ready-to-implement label.
Added Triaged label.

@DartBot

This comment has been minimized.

Copy link
Author

commented Dec 3, 2014

This comment was originally written by bl...@google.com


So where should they be added instead then?

@DartBot

This comment has been minimized.

Copy link
Author

commented Dec 3, 2014

This comment was originally written by @mezoni


So where should they be added instead then?

As workaround you can use package "queries":

================
import "package:queries/collections.dart";

void main() {
  var products = [];
  products.add({"vendor": "Intel", "name": "Celeron"});
  products.add({"vendor": "Intel", "name": "Atom"});
  products.add({"vendor": "Intel", "name": "Pentium"});
  products.add({"vendor": "Intel", "name": "Core"});
  products.add({"vendor": "AMD", "name": "Sempron"});
  products.add({"vendor": "AMD", "name": "Athlon"});
  products.add({"vendor": "AMD", "name": "Phenom"});
  var col = new Collection(products);
  dipslay(col);
}

void dipslay(Collection data) {
  var grouped = data.orderByDescending((e) => e["vendor"])
      .thenByDescending((e) => e["name"])
      .groupBy((e) => e["vendor"]);
  for (var group in grouped) {
    print(group.key);
    for (var product in group) {
      print(" " + product["name"]);
    }
  }
}
================

Output:

================
Intel
 Pentium
 Core
 Celeron
 Atom
AMD
 Sempron
 Phenom
 Athlon
================

It sometimes even slightly faster than Dart <Iterable>.

@lrhn

This comment has been minimized.

Copy link
Member

commented Mar 24, 2015

Added NotPlanned label.

@pulyaevskiy

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

Sorry for bringing this back up.
Surprised to see this declined. Dart has very good standard library for collections and absence of things like groupBy is a little bit unexpected in this regard.

What are the chances of this being resurrected? Or what needs to happen in order to get this back in the "Planned" list?

@lrhn

This comment has been minimized.

Copy link
Member

commented Nov 25, 2015

Depends on what we are allowed to change for Dart 2.0.
Changing the Iterable class at this point is not going to work well (it would have, if people implementing Iterable classes all used IterableBase as the base, but that doesn't seem to be the case).

So, an solution at this point would be adding a separate groupBy(Iterable elements, key(element)) function to, say, package:collection, or using one of the other packages that implement similar functionality.

@pulyaevskiy

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

Thanks @lrhn for the details!

I understand that dealing with breaking changes may be very difficult.

What I wanted to point out is that there is one thing (among others) which is done very well in Dart - it is standardizing of best practices in programming languages and implementing them in the Dart language itself or the standard library. And that's what people refer to a lot when talk about Dart - especially when talk about Futures, Streams and await/async. There is only one way to work with Futures or Streams and it enables great interoperability between all the packages built by the community.

And that is a huge benefit. I think collections library in the core is also one of such features.

Things like groupBy are pretty common and there is already number of packages which introduce their own implementations (queries, dict, dartmixins, to name a few). And this number is going to grow, people will come up with more and more implementations.

I think even this proposal:

adding a separate groupBy(Iterable elements, key(element)) function to, say, package:collection

is a lot better then relying on the army of other packages.

However maybe there is a better way to deal with this? Maybe introduce gradual deprecation policy consisting of several steps like:

  • people who implement Iterable should switch to IterableBase
  • forbid implementations of Iterable outside of dart:core which will enable future improvements
  • introduce new methods in Iterable

Of course, I'm not saying this is the way to go (or practically possible) since I'm not familiar with the development process in the Dart team. But I also can't see in this thread that this issue has been given enough attention.

Regardless I still believe the Dart team has been really great in carefully designing every little piece of the language and the sdk. But maybe this issue deserves to get another round.

@a14n

This comment has been minimized.

Copy link
Contributor

commented Nov 25, 2015

Java 8 solved the problem of new methods in API with default methods. It could be useful to have something similar in Dart to avoid needing a breaking change if you want to add a method.

This issue was closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.