Add .toMap extensions for tuples/pairs and key+value records

[tjarvstrand]

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Since records were introduced in Dart, they have become a much simpler way of representing a list of keys and values than MapEntrys.

This change adds an easy way for users to turn them into maps.

[kevmoo]

You'd need to add a changelog entry and a test here, please.

Thoughts @devoncarew ? @natebosch ?

[natebosch]

We've discussed makingMapEntry an extension type on a record with 2 values which would make these unnecessary . cc @lrhn

I'm a little skeptical of shipping utilities that bridge records and map entries before then. Can you talk a little about your intended use case? Where are you getting an Iterable of records that is sensible to turn into a map?

[lrhn]

My main problem with MapEntry is that object patterns require you to write for (var MapEntry(:key, :value) in someMap.entries) ....
The MapEntry is verbose and obvious.
If we had, fx, for (var .(:key, :value) in someMap.entries) ... instead, I would be perfectly happy about MapEntry.

I've considered changing MapEntry to a record ({K key, V value}) or an extension type on that record (since records don't have constructors), but since the extension type can't implement a record type, it's not as big a help.
The biggest advantage of an extension type on a record would be the structural typing, instead of being a generic class.

Records are not simpler than classes. They're a little diffent. That can be both good and bad.
For example, you can create an extension type that wraps a MapEntry and implements MapEntry. You cannot do that with a record type.

[lrhn]

I'm not keen on just using a positional pair as the representation of a key/value pair.
I'd use a type like ({T1 key, T2 value}) instead. I can see that there is a version below that does that, so I would just drop this one.

It does get a little more verbose if you don't want to use key and value as the names for the key and value.
And if you do have a list of pairs, then having to do .map((p) => (key: p.$1, value: p.$2)) does feel unnecessary.

(Maps are not sets of pairs. Now that we have records, Set<(...,...)>s are sets of pairs!)

[lrhn]

(Just as style comments, not endorsing the functionality:
toX functionality like toMap() is usually a method, not a getter.
The for loop can destructure: {for (var (key, value) in this) key: value}.
This package doesn't use final for local variables.
I'd use "Creates" instead of "Returns" in the comment. Never start a DocComment with "Returns".)

[tjarvstrand]

Can you talk a little about your intended use case? Where are you getting an Iterable of records that is sensible to turn into a map?

I feel like I have to do this all the time when transforming data, which is a lot of what we do as software developers. One prominent example that has come up a lot for me recently is when converting GraphQL data (which doesn't have a Map-type, so instead relies on lists of objects) into a more idiomatic Dart representation.

Records are not simpler than classes. They're a little different.

I approach this from a pure developer experience/ergonomics point of view. I would argue that from that perspective, something like:

  myList.map((element) => (element.x, element.y)).toMap;

is indeed simpler to write and parse than:

  Map.fromEntries(myList.map((element) => MapEntry(key: element.x, value: element.y));

It would of course be even nicer if we had the shorthands available in Scala (element.x -> element.y) or Kotlin (element.x to element.y), but tuples are the next best thing.

I'm not keen on just using a positional pair as the representation of a key/value pair.

This is your taste, which is totally fine. Why does that have to mean you are against providing convenience for others that may prefer something else?

It does get a little more verbose if you don't want to use key and value as the names for the key and value.
And if you do have a list of pairs, then having to do .map((p) => (key: p.$1, value: p.$2)) does feel unnecessary.
In my experience the toMap from a list of pairs usually comes up when the .map is a little more involved. Consider something like this:

Map<UserId, User> myFunction(...)
  ...
  return apiUserList.map((apiUser) {
    final domainUser = decodeDomainUser(apiUser);
    return (domainUser.id, domainUser);
  }.toMap

(Maps are not sets of pairs. Now that we have records, Set<(...,...)>s are sets of pairs!)

Indeed. I'm not sure I understand this comment though. There are a lot of things that can be reasonable to create maps from, depending on your use case 🙂

[lrhn]

When creating a map, there is also the option that I'd recommend instead:

{for (var e in myList) e.x: e.y}

The need to work with MapEntry is only really there when destructuring a map. When creating it, a map literal is almost always better. I'm working on deprecating all the Map.from constructors, possibly other than Map.fromIterables, and maybe a zip would make that case unnecessary too.

If we had a shorter object destructuring, I don't think I'd need more. Going back and forth between MapEntry and pair is a detour.

[tjarvstrand]

When creating a map, there is also the option that I'd recommend instead:
...

Unfortunately this approach is not very conducive to fluent/functional style code.

When creating it, a map literal is almost always better.

Could you elaborate on this? Why is it better?

I would be fine with using literal one-entry maps too, if there was an easy way to turn a list of maps into a single map in a fluent/functional style. E.g.: myList.map((element) => {element.x: element.y}).toMap;

If we had a shorter object destructuring, I don't think I'd need more. Going back and forth between MapEntry and pair is a detour.

In this case, I think having lambda shorthands would be even more handy. Then a general .toMap-extension on Iterable could actually read nicely. E.g., using Kotlin's syntax, it could look something like:myList.toMap(key: $1.x, value: $1.y).

[lrhn]

Could you elaborate on this? Why is it better?

IMO, it's usually more readable. All the code is right there, the entry elements show the actual key and value directly.

That is compared to constructors, which are not conductive to fluent style either.

A toMap() operation would be a way to call such a constructor as a selector. That may be more readable, if part of a long chain, but I'm not convinced that long chains of "fluent code" is actually as readable as the author often thinks. It's easy to understand code you've just written yourself. As a third-party reader doing code reviews of such fluent code, there is a definite "golden hammer" problem, where something that would be trivial imperative code becomes a non-trivial map/where/expand/nonNulls chain.

If the example using .toMap() would have followed it with more chained operations, so:

myList.map(toEntries).toMap().somethingMore();

I can see why .toMap() would seem easier, but:

• You rarely do more selectors on a newly created map. The only ones which wouldn't throw something away is entries, and then creating the map was really just an indirect way of deduplicating keys, it's not used as a map. (There are other derived operations, but they all effectively work off entries.)
• You can still write: {for (var element in myList) element.x: element.y}.somethingMore().

Writing fluent code is not a goal. It's a means to expressing data flow. When the data structure starts mattering, like creating a map, it's no longer just data flow. Or said differently: If the result is not an iterable, it's a good place to break the selector chain.

(But that's just, like, my opinion.)

[tjarvstrand]

IMO, it's usually more readable. All the code is right there, the entry elements show the actual key and value directly.

Makes sense.

A toMap() operation would be a way to call such a constructor as a selector. That may be more readable, if part of a long chain, but I'm not convinced that long chains of "fluent code" is actually as readable as the author often thinks. It's easy to understand code you've just written yourself. As a third-party reader doing code reviews of such fluent code, there is a definite "golden hammer" problem, where something that would be trivial imperative code becomes a non-trivial map/where/expand/nonNulls chain.

I usually find this style to be a lot more readable, so I think it has a lot to do with familiarity and/or taste.

Writing fluent code is not a goal. It's a means to expressing data flow. When the data structure starts mattering, like creating a map, it's no longer just data flow.

I agree that it is not a goal. I just tend to think it usually makes for more readable code.

I didn't come here to argue about the virtues of writing fluent-style code. It is however a common style in the wild. Since collection is so central to the ecosystem, I think it would be nice if it catered to those who prefer that style, even if it may not preferred by its authors.

I'm honestly sad to see that such an unobtrusive and simple contribution just be shot down based on opinions about style that you assume to be universal. The built-in capabilities for collection processing in Dart (even with collection) are very basic, and there is so much that collection` could do to make developers' lives easier but this attitude is not going to take you there. I, for one, am not likely to contribute to this library again.

[lrhn]

I definitely do not assume my opinions on style are universal. I do try to keep a consistent and conservative style for the platform libraries and associated packages, which often means not adding something even though a lot of people may like it. There are many many APIs that a large group of people would like, there is just not a consensus on precisely which APIs, and adding all of them is a non-starter.

So, conservative, and trying to not add something that we'd end up regretting in the future.
(Like almost all the Map.from constructors, which were made unnecessary by more powerful map literals.)

I'd be willing to consider toMap() on Iterable<MapEntry<K, V>>, because it's very clear what it does.
But I'd be even happier to make it completely unnecessary to ever create intermediate MapEntry objects to achieve the same result, or any result, so that the only use of MapEntry will be to iterate a map's entries.
That's the end goal.
I want to avoid encouraging you to create intermediate allocations, which are then immediately destructured and discarded again.

[natebosch]

something like:

  myList.map((element) => (element.x, element.y)).toMap;

is indeed simpler to write and parse than:

  Map.fromEntries(myList.map((element) => MapEntry(key: element.x, value: element.y));

Another option without the intermediate MapEntry is

Map.fromIterable(myList, key: (e) => e.x, value: (e) => e.y);

[mosuem]

Closing as the dart-lang/collection repository is merged into the dart-lang/core monorepo. Please re-open this PR there!