On Fri, 27 Mar 2009, Bent C Dalager wrote:
On 2009-03-27, Tom Anderson <twic@urchin.earth.li> wrote:
Personally, i'd also like to see Map.get throw a KeyNotFoundException
or some such instead of returning null when a key isn't in the map,
but i think most people wouldn't agree with me on that.
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.
actual value per key can be different. This is often very useful:
// key not found in backing map.
return an Entry object, or to at least have a getEntryFor() method.