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

Make maps not always use null as default value. #1005

Open
lrhn opened this issue Jun 4, 2020 · 14 comments
Open

Make maps not always use null as default value. #1005

lrhn opened this issue Jun 4, 2020 · 14 comments
Labels
feature Proposed language feature that solves one or more problems

Comments

@lrhn
Copy link
Member

lrhn commented Jun 4, 2020

Currently Dart maps return null when a lookup fails. With null safety, that means operator[] always returns a nullable type. That can be annoying.

Consider if Map had three type parameters: K, V and D where D is the type of the default value returned when a lookup fails.
It would be Map<K, V extends D, D>, and operator[] has return type D.

To avoid breaking everyone we could introduce a new Map type, XMap (probably not), and have all current Map<K, V> instances implement XMap<K, V, V?>.

Then we can have XMap constructors taking a default value, rather than just always using null as the default value.
Map literals could write {"x": 1, "y": 2, default: -1} to create an XMap.

Obviously we'd want to rename XMap to Map soon enough, but without breaking code which only knows the old Map. That could be based on language version, and maybe type aliases.
Old code sees typedef Map<K, V> = real.Map<K, V, V?>;, new code sees the real Map directly.
(Maybe they'll want a some short type aliases too, like typedef map<K, V> = Map<K, V, V>; )

@lrhn lrhn added the feature Proposed language feature that solves one or more problems label Jun 4, 2020
@mateusfccp
Copy link
Contributor

I'm looking forward to be able to do a Map<K, Option<V>> with default: const None().

@Cat-sushi
Copy link

I feel -1 like yet another null without any language support.

@mateusfccp
Copy link
Contributor

mateusfccp commented Jun 4, 2020

I feel -1 like yet another null without any language support.

He is only giving a example of the default syntax for a literal Map. He is not saying -1 is the default for integer maps or anything like this.

@Cat-sushi
Copy link

I'm also giving an example. My point is that null is better than other default values, because null has many supportive futures such as ?, !, non-null promotion with flow analysis and most importantly null pointer exception (no such method error).

@leafpetersen
Copy link
Member

I wonder whether this really covers the common case? It seems to me that there are two kinds of indexes into a map:

  • I know that key in the map, and want its value
  • I don't know whether key is in the map

In the first case, users should prefer not to get a nullable return type, but should be mostly agnostic between throwing an exception or returning a default value (though they may prefer to get an exception since if they have a bug, the exception will surface).

In the second case, it's not clear to me what users will mostly want.

  • If the user doesn't actually want to use a default value, but instead has to check the return see if it is the default value, they are strictly worse off than in the nullable case.
    • They have to know what the default value is, which means knowing where the map was produced.
    • Unless the default value is null, they can't use existing features that make working with null easier (e.g. a[k] ?? b)
  • If the user does want to use a default value, they may not want to use the default, so again they are back in the previous scenario of checking for the default value
  • If the user does want to use the one single default value provided by the map, then this is the right thing.

So it's really not clear to me that getting a single default value is what users will want most of the time. I actually suspect that we would cover more use cases if we added a separate operator (maybe use the call operator) which indexed and threw if the value was not there. But really, this feels like something that would be nice to get data on.

@samandmoore
Copy link

samandmoore commented Jun 17, 2020

I agree @leafpetersen.

In Ruby, a language I've spent a lot of time with, they support square brackets for "gimme the value or null" and fetch(key) for "gimme the value or throw." And then they have an optional block parameter for fetch that lets you provide a default value, like so: map.fetch('foo') { 'default' }

I wonder if that sort of API would feel good to dart users. I know that I would like it and it's not that different from the methods on Iterable that take orElse. In dart it could even be: map.fetch('key', orElse: () => 'default').

Using a lambda is a nice way to allow for new instances of the default value to be used on each fetch. For example, if you set the default value at Map construction time and you use a type like List, then each time you'd have a key miss, would you get a new List or a reference to the one default List instance created at Map construction time? If it's the latter then ant mutations to that List will likely result in surprises for other key misses that return a non empty List.

Anyway, I think a fetch method might be a good alternative approach.

@leafpetersen
Copy link
Member

cc @pq @bwilkerson I just noticed in passing that my last comment here was relevant to our discussion about gathering data. Could be a kind of interesting test case. How many uses of the map index operator:

  • Are used in a way that assumes they are not null (method call on them, etc)
  • Are used in a context that checks for null and replaces with a default (a[k] ?? d)
    • And of these, how often is d a constant
  • Can't be assigned to one of the above categories.

There are other interesting things you could look for, but they might be harder to track, e.g.
- Are assigned to a variable that is then checked for null and used to choose different code paths based on null/not-null

@leafpetersen
Copy link
Member

Is this still under consideration?

@lrhn
Copy link
Member Author

lrhn commented Sep 11, 2020

I'd keep it open as an enhancement request, but it's not NNBD-specific.

@MarvinHannott
Copy link

MarvinHannott commented Jan 31, 2022

@leafpetersen The biggest problem with returning null is that it doesn't tell us if there wasn't a value or if the value indeed was null. It's like returning 0 for a failed string to integer conversion in C.

Indeed, I have run into multiple instances where not considering that a generic type could be null leads to very subtle bugs, for example when implementing Iterator.

With null safety, when you have a type T?, you somehow expect it to be a nullable type that could also hold a value of null without being null; or in other words, you expect it to behave like an T??; Because how else can we tell these cases apart. I hope that makes sense 😅

@munificent
Copy link
Member

With null safety, when you have a type T?, you somehow expect it to be a nullable type that could also hold a value of null without being null; or in other words, you expect it to behave like an T??; Because how else can we tell these cases apart. I hope that makes sense 😅

It does make sense. This is an unfortunate but known limitation of using nullable types instead of option types. I wrote about this at length here. It's a trade-off. Dart can't represent nested nullable types like you might like in cases like this, but in return other idioms get simpler and easier.

@Levi-Lesches
Copy link

Also, you can tell the difference: Map.containsKey

Map<String, int?> numbers = {"1": null, "2": 2, "3": 3};

bool contains0 = numbers["0"] == null;  // true
bool contains1 = numbers["1"] == null;  // also true
bool contains2 = numbers.containsKey("2");  // unambiguously true

@MarvinHannott
Copy link

MarvinHannott commented Feb 1, 2022

Also, you can tell the difference: Map.containsKey

But then you perform two operations, one of which is unnecessary. I presume that Map.containsKey() involves hashing or linear search?

Even if it doesn't, you have to perform another (unnecessary) null check. Then I could argue that map[] shouldn't return null, but throw an exception, just like array.firstWhere() does (which also highlights some api inconsistency).

Alright, this was a bit beside the point, which was that not considering this case of null not being absence carefully enough can lead to very subtle bugs. I mean, it is a case very easily overlooked.

@munificent Well, in the end, this problem can easily be mitigated by a very simple class like

class Absent<T> {
  T? get value; 
  bool get isAbsent; 
}

where Absent (not the best class name) is something like JavaScript's undefined (now I finally get, why undefined was ever considered useful). Maybe this is worth putting in a library, or even dart:core, because I think of it somewhat essential. Essentially any library out there with Optional types just uses null to represent absence, which isn't quite right.

@lrhn
Copy link
Member Author

lrhn commented Feb 1, 2022

For the record, if I was to design Map today, knowing what we know about null safety, I'd probably either make operator[] throw on missing keys, or treat storing null the same as remove and perhaps even make V non-nullable.
The current design for Map was born out of Java's Map interface, and that was based on everything being nullable. It does make some things easier that null is just implicitly included in all types, keeping the program error free is not one of them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems
Projects
None yet
Development

No branches or pull requests

8 participants