This seems very situational. In some use cases, a Map receiving lookups
of entries it doesn't have is considered entirely normal (a cache
perhaps) and so shouldn't be handled as an exception (and having to
first do a boolean lookup of the type "isEntryPresent" only then to call
get() on the same entry seems wasteful) and a null return value is both
useful and proper. In other uses you only look up values you know should
be there and in these cases a miss should be handled as an exceptional
condition.
I certainly agree that both these use cases are valid (although i'd bet
the latter is ten times more common), but i don't see why having a null
return is necessary to address the former. This code:
Thing value = map.get(key);
if (value != null) {
dealWithValue(value);
}
else {
dealWithLackOfValue();
}
Is trivially rewritten as:
try {
Thing value = map.get(key);
dealWithValue(value);
}
catch (KeyException e) {
dealWithLackOfValue();
}
With no loss of readability. There is a potential performance penalty, but
i don't believe it would be significant with modern VMs.
Many people will dislike the latter version because it 'abuses'
exceptions. I think this betrays an overly dogmatic attitude to
exceptions, but it's a matter of taste.
The thing that most motivates me to want the latter version is that it
makes it impossible to ignore absent keys. With a null return, code like
this:
class Customer {
private static Map<Locale, Country> COUNTRIES_BY_LOCALE;
private Country country;
public Customer(Locale locale) {
country = COUNTRIES_BY_LOCALE.get(locale);
}
}
Effectively fails silently; when there is no country for that locale in
the map, the customer gets a null country, and the code continues. Later,
at some distant point in the code, things go tits-up because of that null
country. Yes, you could add a null check in the constructor, but that's an
extra line of code that has to be remembered to be written. Because
Map.get fails silently, it has to be written *every* time that method is
called! If Map.get threw an exception on a missing key, it would be much
harder to ignore - it would take active work to write a catch-and-ignore.
The distinction can be handled by having different Map classes for the
different use cases, by having a state the Map can be put in (do/do not
throw on miss) at creation or even dynamically,
Horrible!
The Right Thing to do would be to have two methods, get (which throws an
exception) and getIfPresent (which returns null). Or get(K), and get(K,
V), where the second version takes a default value to return if the key
isn't in the map - often much more useful than a null return. Of course,
this is a change which would be impossible to retrofit, because it would
break existing code - this would have to have been done when Map was first
written. But then, any change to the semantics of Map.get at this point
falls foul of the same trap.
or by having a wrapper layer that changes the default (currently "return
null on miss") to whatever you desire. The latter is probably the easier
to introduce retroactively although it may have to throw an unchecked
exception.
That's a good idea - it would be another Collections.somethingMap method.
The wrapper would probably have to disallow null values in order to work
efficiently. And you could have a Collections.defaultingMap which does the
return-a-default thing, althought the default would be fixed per instance
rather than per call.
I don't see that either behaviour is "naturally" more correct than the
other across all use cases however.
That's true. I'd rather have both than either one, for sure.
The default should probably be chosen based on how easy it is for a
wrapper to painlessly convert it into the other behaviour.
It's not possible to convert the current behaviour to what i want unless
you know that there are no null values in the map. Not without calling
containsKey every time, anyway.
tom
