dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.23k stars 1.58k forks source link

Separate Map-like queryability from Map edit operations #56469

Open mimbrown opened 2 months ago

mimbrown commented 2 months ago

The Problem

  1. Maps are very common and useful data structures for modeling things.
  2. Dart provides some nice syntax sugar for maps (e.g. spread operator).
  3. People like immutable types, including immutable maps.
  4. Key point: there's no way to define something that's meaningfully "map-like" according to Dart without also implementing mutating methods

Proposal

Add the equivalent of an Iterable for Maps. For the sake of this let's call it MapQueryable (not a very good name). This separates out the concept of "queryable like a Map" (with standard methods like containsKey, containsValue, map, operator [], and getters like keys, values, entries, length, etc), from an actual Map with all its methods.

So this would mean:

  1. Add the MapQueryable interface
  2. Update Map to implement the interface
  3. Update methods everywhere which only read from the passed map to accept MapQueryable instead of Map.

Advantages

  1. Cleaner separation: things can be "map-like" without implementing mutation
  2. Which also unlocks syntax sugar like the spread operator applying to a wider class of user-defined objects
  3. Stricter guarantees: if there is an API which takes a MapQueryable instead of a Map, I as a consumer of that API can be certain that my map will not be mutated when I pass it.
mateusfccp commented 2 months ago

Map is actually an interface (abstract interface class Map<K, V>), and you can implement it as you like.

You can also spread your map implementations into map literals with no problem.

I'm not sure how it would work with non-mutating maps, tho. The Map interface has type parameters for keys and values, but not for something like the returning type of changing-like functions. Also, by language definition, []= returns void, and even though void is a supertype, you can't do something like final newMap = (immutableMap[key] = value) as MyMap<K, V>.

lrhn commented 2 months ago

What unmodifiable and constant maps do is to throw (UnsupportedError) when calling a mutating function. That's the recommended approach.

Having an interface with only the read-methods allows a method to say that it expects to only read from an argument. It can still lie and do is Map, so it's not a guarantee.

I don't expect to change the current design choice of not having read-only interfaces for collections

mimbrown commented 2 months ago

What unmodifiable and constant maps do is to throw (UnsupportedError) when calling a mutating function.

In my opinion that's a sign that there is some abstraction missed here, especially if it's normally the same set of methods which are left unsupported. It's nicer to catch things at compile/lint time than runtime.

It can still lie and do is Map, so it's not a guarantee.

True, "certain" was too strong a word. But still, it would be a much better indicator of the method's behavior than that just taking a Map.

I don't expect to change the current design choice of not having read-only interfaces for collections.

That's the thing though, Iterable already sort of fills this gap for Lists (I know there's plenty of other things that implement Iterable, so it's not apples to oranges). I don't see any inherently mutating methods in Iterable (might be missing something). It's also not just about mutability, it's the ability to iterate over the thing like a Map. I called it MapQueryable but probably a better name would be KeyValueIterable.

mimbrown commented 2 months ago

What unmodifiable and constant maps do is to throw (UnsupportedError) when calling a mutating function.

Actually, tbh, I feel like this is one of the most dangerous aspects of Dart that I frequently come up against. The fact that const is generally recommended wherever possible, but the compiler can't always catch that some function is trying to mutate a map originally declared with const (possibly in some rarely-used branch), is itself an issue that should be solved. I'm not at all sure what this would entail, but an extension to this proposal would be to treat maps declared with const as a KeyValueIterable instead of a Map, which would automatically give some much needed type-safety to these situations (I'm aware that I'm skipping over the immense pain of converting everything that currently takes a Map but doesn't mutate it to now take a KeyValueIterable).