please extend for-in syntax to maps

[trinarytree]

when xs is an Iterable, the syntax for (var x in xs) is available inside both imperative code and list/set/map literals.
it would be very useful to extend this syntax to maps.
examples:

for (var key, value in map) {      // notice how 2 vars (key and value) are declared at once
  print('$key: $value');
}

var otherMap = {
  for (var key, value in map)
    key + 1: value * 2
};

var list = [
  for (var key, value in map)
    key + value
];

var set = {
  for (var key, value in map)
    key + value
};

the current workarounds hurt readability and/or efficiency:

// hard to read
for (var ssnAndPerson in ssnToPerson.entries) {
  print('${ssnAndPerson.key}: ${ssnAndPerson.value}');
}

// easy to read
for (var ssn, person in ssnToPerson) {
  print('$ssn: $person');
}

// cons:
// - ok for imperative code, useless for list/set/map literals
// - can't use break, continue, return
ssnToPerson.forEach((ssn, person) {
  print('$ssn: $person');
}

// another common workaround.  cons:
// - repeats the map name
// - needs a pointless ! for null-safety
// - inefficiently looks up key in the map
// - no nice name for the value unless one wastes a line putting it into a local var
//   which isn't an option inside a list/set/map literal
for (var ssn in ssnToPerson.keys) {
  consume(ssnToPerson[ssn]!, ssn + offset);
}

// ugh!!
Map<int, int> plus1(Map<int, int> map) {
  var otherMap = <int, int>{};
  map.forEach((key, value) {
    otherMap[key] = value + 1;
  });
  return otherMap;
}

// elegant
Map<int, int> plus1(Map<int, int> map) =>
    { for (var key, value in map) key: value + 1 };

// similar problems to "another common workaround" above
Map<int, int> plus1(Map<int, int> map) =>
    Map.fromIterable(map.keys, value: (key) => map[key] + 1;

[lrhn]

I don't think we'll do this until we have patters or records (current proposals: https://github.com/dart-lang/language/blob/master/working/0546-patterns/patterns-feature-specification.md, https://github.com/dart-lang/language/blob/master/working/0546-patterns/records-feature-specification.md).

At that point, we could possibly allow pattern matches in the binding of the for/in, like:

for (var (key: k, value: v) in map.entries) { ... }

where var (key: k, value: v) is a binding pattern mathching the MapEntry class. (Or, preferably, change MapEntry to just be a record of type (key: K, value: V) for a Map<K, V>. The same pattern would apply, as far as I understand it (which is hardly a guarantee).

With views, we could even make MapEntry a view on a positional record/tuple, (K, V), and then, depending on the assignability of views to their underlying type, you might be able to do for (var (k, v) in map.entries) ....

@munificent
Should/does the binding in a for/in allow the same patterns and assignments as a normal declaration?

[trinarytree]

@lrhn thanks for the links. great to see destructuring in the works.

while i realize destructuring may allow syntax like

for (var (key: k, value: v) in map.entries) { ... }

it's very noisy compared to the proposal.
this would involve extra parentheses, colons, the words "key" and "value" and "entries".
even if such destructuring is happening under the hood, it would still be great to have syntactic sugar
that eliminated all of that noise.

maps are one of the most common data structures,
so even small gains in their convenience/readability has huge impact.
to show how dart already embraces this philosophy, in the following syntax,

{ for (var x in xs) x: f(x) }

the expression x: f(x) is not allowed just anywhere, but it's allowed here because it makes
working with maps easier, and it's great.
one could have only insisted, for the sake of making for loops more regular,
that the expression after the for loop be a "normal" expression,
the kind that would be allowed on the RHS of an assignment, but then we'd have a mess like this:

<X, Y>{ for (var x in xs) MapEntry(x, f(x)) }

btw, currently for-in loops don't allow declaring > 1 loop control variable:

for (var x, y in xs) {}   // syntax error: unexpected token ','
var x, y;                 // allowed

so i don't think the proposal has any conflict with existing syntax.

in terms of intuition, if someone wanted to iterate over map, not map.entries, but just map,
what would you think they meant?
python comes close to this level of elegance:

for k, v in map.items():
  print(k, v)

but it still forces you to say .items(),
(imo) oddly taking the asymmetric stance that iterating over a map means iterating over the keys.
i would think the concept of iterating over a data structure should mean
to get all of the data out of it, not just some.
i think dart can do better.

[lrhn]

It's definitely possible to introduce a special for (varOrFinal id1, id2 in expression) ... which works only on Maps by iterating over their .entries (basically desugaring to for (var (key: id1, value: id2) in (expression).entries) ...).

Whether it's worth the extra complexity over just iterating the entries directly, is what we'll have to decide.
(Or whether there is a better syntax - do we want to allow for (int key, String value in <dynamic, dynamic>{1: "a"}) ...?)

[jodinathan]

indeed a simple for (varOrFinal id1, id2 in expression) ... syntax sugar would be very nice @lrhn

[Jetz72]

If for(var key, value in map) is on the table, any chance something like for(var index, item in list) could fit in with that? There's the ListExtensions forEachIndexed and the classic for(int i = 0; i < list.length; i++) {var item = list[i];..., but it'd be nice to have a more elegant shorthand for that.

[julemand101]

@Jetz72 If this new loop for maps got added you could do:

for (var index, element in someList.asMap()) {
  print('$index: $element');
}

[munificent]

At that point, we could possibly allow pattern matches in the binding of the for/in, like:

for (var (key: k, value: v) in map.entries) { ... }

where var (key: k, value: v) is a binding pattern mathching the MapEntry class. (Or, preferably, change MapEntry to just be a record of type (key: K, value: V) for a Map<K, V>. The same pattern would apply, as far as I understand it (which is hardly a guarantee).

With views, we could even make MapEntry a view on a positional record/tuple, (K, V), and then, depending on the assignability of views to their underlying type, you might be able to do for (var (k, v) in map.entries) ....

@munificent Should/does the binding in a for/in allow the same patterns and assignments as a normal declaration?

Yes, the patterns proposal does support using patterns in for-in variable declarations. The exact syntax you have up there would work. As would the slightly shorter:

for (var (key:, value:) in map.entries) {
  // Local variables "key" and "value" here...
}

Having to extract the key and value by name is a drag, as is having to call .entries explicitly. I can think of three core library changes to improve that:

1. We could make MapEntry implement Destructure2. That would let you do:

   for (var (k, v) in map.entries) {
     // Local variables "k" and "v" here...
   }

   It lets you destructure positionally, but still requires the .entries. I think this is non-breaking.

2. We could make Map<K, V> implement Iterable<(K, V)>. That would let you do:

   for (var (k, v) in map) {
     // Local variables "k" and "v" here...
   }

   That looks great to me and doesn't require any language changes. It's a breaking core library change, though. :(

3. We could change the language to allow for-in to desugar to calling .iterator as an extension. Then we add an iterator extension on Map<K, V> that returns an Iterable<(K, V)>. This is non-breaking to the core libraries, but requires a language change.

Personally, I'd lean towards 3.

[mateusfccp]

@munificent Would that mean that we would be able to implement iterator on any arbitrary class and then use it with for-in? That would be nice.

[lrhn]

We can't make Map implement Iterable sincce they disagree on a number of members, like forEach. (Well, unless void Function(K, V) and void Function((K, V)) are isomorphic, which would be awesome, but probably problematic).

We don't currently have an interface that only contains Iterator<T> get iterator. The "iterate extension iterator" approach would be better.

I do not like Destructure2 and its ilk.O
We could make MapEntry<K, V> a view on (K, V) with assignability to (K, V), and then you can treat Iterable<MapEntry<K, V>> as Iterable<(K, V)>. That would be neat.

If we had had records when MapEntry was introduced, I'd probably have made MapEntry a record.
If it had been a named record, (key: K, value: V), then it's because people are intended to use named destructuring.
if it was positional, (K, V), then people would be supposed to understand the ordering, and it's OK to use positional destructuring.
If we introduce records now, we should follow the same approach and make it either a positional record or a named record, and only support destructuring in one way, not two.

It probably would have been named if other people had a say, a lot of people seem to like being explicit over being brief. I'd go for positional myself. The name isn't needed, and if you're ever in doubt whether it's (key, value) or (value, key), ... well, you won't be, or the types will set you straight.

(Could we have "optionally named positional record entries"? Say (int x, int y, z: int) which is just the same as (int, int, z: int), just with a little extra metadata in the static type which allows you to write record.x as an alias for accessing the zeroth positional entry. I made a suggestion like that for function calls once, where a static type of void Function(int x, int y) would be a function with two positional arguments, but where you can write f(y: 1, x: 2) to name the positional arguments if you want to. Purely static information which affects how you can write positional arguments at call-points, no effect on subtyping and not reified at run-time.)

[Wdestroier]

I'd vote for non-named arguments, because key and value are names that don't help to understand the code.

Non-named arguments example:

for (final (countryCode, countryName) in countries) {
  if (countryCode == 'BR') {
    print('I live in $countryName');
  }
}

Named arguments example:

for (final (key:, value:) in countries) {
  if (key == 'CU') {
    print('I live in $value');
  }
}

[lrhn]

It's interesting that we sometimes consider key and value important roles for the entries of a map, and at other times we don't.
Here @Wdestroier uses (countryCode, countryName) for the key and value, because they know that the values that are used as keys and values wrt. the map also have different roles in another setting. (And maps a usually used as part of some larger data model, not as entities by themselves, so the keys and values almost always have separate meanings.)

I still think we should wait for pattern matching and records before we introduce a new language feature for a problem that could potentially be solved by just iterating over an iterable of pair-records.

[lrhn]

My current long-term goal: Wait for inline classes (#2626), make it possible for them to implement non-inline-class types of their representation type, then change MapEntry to be declared as:

inline class MapEntry<K, V> implements (K, V) {
  final (K, V) _pair;
  MapEntry(K key, V value) : _pair = (key, value);
  K get key => _pair.$0;
  V get value => _pair.$1;
}

That should make MapEntry be a pair (two-tuple, a record), but viewed as a MapEntry-class like value.
It'll even be a subtype of (K, V), so you can do:

for (var (k, v) in map.entries) { ... }

If we get class modifiers before inline classes, I'll try to make MapEntry final first, to prevent people depending on the current class.

Let's see how far we can get towards that. (Just being a subtype of (K, V) is enough for this.)

[trinarytree]

for (var (k, v) in map.entries) { ... }

is a big improvement, but the .entries part mars it.
it's extra fluff that you have to memorize when writing code and ignore when reading code.
can we go the final step and get this?:

for (var (k, v) in map) { ... }

[lrhn]

@trinarytree

The problem with allowing for (var (k ,v) in map) ... is that this is already valid syntax for iterating an iterable of pairs.

We'd have to decide which behavior to use based on the static type of the map expression.
If it's an iterable, use the normal iteration.
Otherwise, if it's a map, iterate map.entries.map((entry) => (entry.key, entry.value) instead.
(If the type is both - if that's even possible without interface conflicts on, e.g., the map method - we use the iterable behavior. So don't make an IterableMap!)

It's doable. It's even doable for dynamic typed map expressions, where we'd do the type check at runtime.

Edited: I did mean entry.value, not entry.map.

[trinarytree]

when you said
map.entries.map((entry) => (entry.key, entry.map)
did you mean
map.entries.map((entry) => (entry.key, entry.value))?

at any rate, if this syntax

for (var (k, v) in map.entries) { ... }

is available, i'm guessing it wouldn't be too hard to have the compiler implement logic like
"if someone tries to iterate over m and m is Map && m is! Iterable, then iterate over m.entries instead."

you hinted at possibly using a static type check to make the decision when the static type is not dynamic.
that creates a slightly strange/ambiguous case if the static type does not implement Iterable but the runtime type does.
but this is such a corner case, i doubt anyone practically cares how the tie is broken.

[lrhn]

Currently, if the static type does not implement Iterable (or dynamic), then you don't get to use for(...in...) at all.

Also, today, if a class can ever implement both Iterable and Map, the for(..in..) would iterate the iterable.
That's why I'd tie-break in that direction, if we ever added the ability to iterate maps directly. It would be non-breaking.

[munificent]

Randal L. Schwartz suggests defining the following extension (which you can do in your own code or we could maybe add to the core libraries at some point):

extension MapExtensions<K, V> on Map<K, V> {
  Iterable<(K, V)> get keyValues => entries.map((entry) => (entry.key, entry.value));
}

With that, you can now write:

for (var (key, value) in map.keyValues) {
  print('$key: $value');
}

var otherMap = {
  for (var (key, value) in map.keyValues)
    key + 1: value * 2
};

var list = [
  for (var (key, value) in map.keyValues)
    key + value
];

var set = {
  for (var (key, value) in map.keyValues)
    key + value
};

If we do #2563, then the extension isn't needed at all and you can just use:

for (var (:key, :value) in map.entries) {
  print('$key: $value');
}

var otherMap = {
  for (var (:key, :value) in map.entries)
    key + 1: value * 2
};

var list = [
  for (var (:key, :value) in map.entries)
    key + value
];

var set = {
  for (var (:key, :value) in map.entries)
    key + value
};

Given that, I think it's unlikely that it would be worth it to add special-case support for iterating over key-value pairs to for-in loops. Patterns and records already (with this use case in mind) work in for-in loops and allow you to bind multiple variables per iteration.

I'm going to go ahead and close this, but it's a great suggestion to draw attention to an annoying wart in the language which should hopefully be much less annoying now.

[trinarytree]

records are an improvement, but needing an extension is annoying. i don't want to have to remember the name of the extension and import it just to do one of the most basic things one ought to be able to do with a map. and since it's an extension, standard tools probably won't autocomplete the method name at least until you remember the name of the import for yourself. so as hinted at, adding this to the core libraries would improve convenience a lot.

similarly, having to use any method name just to iterate over a map is annoying. as i mentioned before,
it's extra fluff that you have to memorize when writing code and ignore when reading code.

i feel like this minor tragedy is because of the history of the development of the language. if records had been there from the beginning, one could have made Map an Iterable and there would have been little need for the 2-param forEach method, for MapEntry, for the entries property, etc.

[munificent]

i feel like this minor tragedy is because of the history of the development of the language.

Every programming language carries the sins of its past. There is no escaping path dependence.