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 .records getter to Map (as an extension method) #54965

Open
Reprevise opened this issue Feb 20, 2024 · 7 comments
Open

Add .records getter to Map (as an extension method) #54965

Reprevise opened this issue Feb 20, 2024 · 7 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug

Comments

@Reprevise
Copy link

Reprevise commented Feb 20, 2024

Given that changing the implementation of MapEntry to a record like ({K key, V value}) would be a big breaking change, I'm opening this issue to see the feasibility of adding a records getter as an extension to the Map class. This would be similar to what was done with indexed on Iterable. This is something I could probably open a PR for myself but I wanted to create this first to get thoughts from the Dart team and others.

In my projects I have it defined as:

typedef MapEntryX<K, V> = (K key, V value);

extension MapRecords<K, V> on Map<K, V> {
  Iterable<MapEntryX<K, V>> get records => entries.map((e) => (e.key, e.value));
}

In the SDK, I would assume this implementation would change to returning a custom implementation of Iterable (like IndexedIterable for Iterable.indexed).

The usage would look like this:

const Map<int, String> namesById = <int, String>{1: 'Bob', 2: 'Steve', 3: 'Julia'};
for (final (key, value) in namesById.records) {
  // ...
}

Of course, this would all be moot if we could just remove the type when destructuring 😉

for (final /*MapEntry*/(:key, :value) in namesById.entries) {
  // ...
}
@dart-github-bot
Copy link
Collaborator

Item Details
Summary Add .records getter to Map (as an extension method)
Triage to area-library

(what's this?)

@dart-github-bot dart-github-bot added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Feb 20, 2024
@lrhn
Copy link
Member

lrhn commented Feb 20, 2024

would be a big breaking change

I hope not, because that's what I plan to do eventually.
Or, more precisely, make MapEntry be an extension type on the record type ({K key, V value}). (Can't make it implement the record type, sadly.)
Then you can safely cast .entries to Iterable<({K key, V value})>, and you can still do dynamic access to key and value.

Of course, this would all be moot if we could just remove the type when destructuring

Yes, please!

@lrhn
Copy link
Member

lrhn commented Feb 20, 2024

About the idea itself (if we can't get rid of the type when destructuring), it's not impossible, but it's also not particularly important when you can just write MapEntry in front and get the same effect.
Which means it's not so important that we're likely to introduce specialized implementations for the platform Map implementations (and a way to opt in to it, which requires some check in the extension method to see if the map is a platform map implementation).

@lrhn lrhn added library-core type-enhancement A request for a change that isn't a bug labels Feb 20, 2024
@lrhn lrhn removed the triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. label Jul 18, 2024
@rakudrama
Copy link
Member

I like the idea of having a lighter-weight pattern when the context provides the type.
Why not

    for (final (id, name) in namesById.entries) {
      //
    }

How might this work? The record-pattern in destructuring is reconsidered as an object-pattern when the subject of descructuring has a type where there is a primary constructor that can fill in the missing parts of the object-pattern (in this case, the class/interface MapEntry).

@lrhn
Copy link
Member

lrhn commented Nov 22, 2024

No constructors needed. Object patterns are not based on constructors.

It would just say that a syntactic record pattern with no positional fields in a declaration pattern, where the matched value type is not a record type, and not dynamic or Never, is an object pattern with the static matched value type as implicit object pattern type.

It'll probably give the matched value expression a context type which is a record type. It just doesn't have to satisfy that context type.

Would be

for (var (:key, :value) in map.entries)...

The alternative, discussed in dart-lang/language#4124, is to have a short marker before the parentheses: .(...) or _(...) were suggested. That marks the pattern as an object pattern on the matched value type, and it can avoid introducing a record context type. It might even be usable in a matching pattern.

@rakudrama
Copy link
Member

No constructors needed. Object patterns are not based on constructors.

I would like a concise way to not having to use the names key and value.
That would require some way of telling if the full pattern should be final MapEntry(key: id, value: name) or final MapEntry(key: name, value: id). I though perhaps a constructor could help match up positions.

@lrhn
Copy link
Member

lrhn commented Nov 24, 2024

Constructors are unrelated to destructuring. There is not necessarily any direct connection between constructor parameters and object state, or between object state and accessors.
If the compiler tries to make such a connection, say by recognizing initializing formals, then it makes an implementation detail semantically significant and makes it a breaking change to change implementation.

The only part of a class that you can rely on being intentional, is its public API. That's why the only thing destructuring can access is getters, and only by name. That's all that the class actually promises. (It should be able to call other methods too, IMO, but still only explicitly declared object APIs.)

If you want to destructure a map entry without writing key and value, then you'll need to convert the entry to a record, like .map((e)=>(e.key,e.value)). That's the only object that is both heterogeneously typed and can be destructured by position.

I don't want to add such a getter to maps. Map entries are not anonymous pairs, maps are not just sets of pairs. The key and value roles are significant.
Getting rid of having to write MapEntry in front, when we already know that the value has that type, would be nice though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-core type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants