persistence
persistence copied to clipboard
Provide API returning Optional as an alternative to methods allowed to return null
issue #298 added getSingleResultOrNull()
method to Query
(and relevant subtypes) to avoid the requirement of explicit exception handling in case no result is found through the call to getSingleResult()
.
This is a follow up issue explicitly asking to support Optional
where appropriate as opposed to supporting it in one particular method for one particular use-case as was asked for in #298.
So here's my reaction to this, FWIW, and I'm well aware that some other people are likely to strongly disagree with what I'm about to say.
When Optional
was first proposed, many of us strongly criticized it as a completely broken and untypesafe ad hoc attempt to implement a Haskell-style Maybe
in Java. [A correct implementation of Maybe
in Java is impossible because Java has no sum types.]
At the time, those proposing Optional
defended it on the basis that no, this is just a little convenience thing for use with the new streams API, and that it wasn't supposed to be a whole new approach to dealing with null
in Java. I was at the time highly skeptical of this claim, but to my pleasant surprise, they've largely kept this promise, and with only a handful of unfortunate exceptions, Optional
has not really leaked into the Java APIs I use every day. Most API developers have (wisely, in my view) chosen to avoid it, and I just don't have to deal with it in my code.
[Ironically, the Java steams API itself has turned out to be far less useful than most—myself definitely included—predicted, and so I don't even really deal with Optional
very often even in that context.]
So, now, why am I saying all this?
Well, we could certainly provide overloaded operations which return Optional
in JPA, but:
- when
Optional
was introduced, we were promised that it was not going to be used in this way, - it makes adds incremental complexity to our APIs without (a) adding new functionality, (b) improving typesafety, nor even, in my opinion (c) enhancing ergonomics, and
- it gives
Optional
an extra beach head from where it can quite easily metastasize into exactly the sort of broken "general purpose" approach to null handling that it was never supposed to be, and for which it is misconceived and mis-designed.
What I'm saying is that JPA is one of the most prominent Java APIs, and in my opinion we should set an example here. Our embracing this abomination would send a signal to other Jakarta projects and other libraries in the Java ecosystem at large that this is the way to go, and in a few years time my code will be filled Optional
.
So that's the case against doing this.
[Another day I'll write up why I think that there is indeed an issue with how EntityManager.find()
treats the unfound case, and why we maybe should indeed do something about that, and why that "something" should not be findOptional()
.]
Anticipating arguments against the view I've stated above, let me state what I think is the best case in favor of the use of Optional
in APIs like JPA:
- For casual users of the API, it acts as excellent documentation of the fact that an operation can return no result.
- It lets you can combine
getSingleResult()
/getSingleResultOrNull()
into one method declaration. - Even if it isn't completely typesafe, it still forces callers to explicitly do at least something about the null case, so users are forced to at least consider it.
Unfortunately, none of those motivations are very convincing in our case where we already have an operation which doesn't return Optional
, and we're considering adding an overload.
- the new overload doesn't do much at all to clarify the null semantics of the original method,
- it doesn't reduce the number of methods, you need
getOptionalSingleResult()
or whatever, and - the sort of lazy user who is inclined to not think about
null
is almost certain to just callgetSingleResult()
instead ofgetSingleResultOrNull()
, especially given the lack of ergonomics of Java'sOptional
. This comes back to my principle that "features for type safety are most useful when they're opt-out, not when they're opt-in".
That's ... pretty devastating to the case for Optional
, IMO, at least in our context.
The direction we went with getSingleResult()
+ getSingleResultOrNull()
looks pretty good by comparison:
- It's just as good at documenting the null semantics.
- It's the same number of methods.
- It's not especially vulnerable to NPEs, since you have to explicitly call the less-convenient form if you want a
null
, and - it's much less verbose and much more ergonomic on the caller side.
Now, OTOH, the case I'm concerned about is find()
. I fear that we messed up the semantics of this method years ago, and that it should throw when the entity is not found, just like getSingleResult()
.
So I would love there to be two versions of find()
, just like there's two versions of getSingleResult()
. Except ... the naming just doesn't work out very well. [FTR, even findOptional()
doesn't work as a method name.]
An idea I've been playing with for quite a long time is to have a non-null version of find()
called load()
. But I still have not managed to convince even myself that this is the right solution.
Outside Persistence I use this convention:
- get goes for something I know it is there -> if it is not, throw an exception
- find searches for something. It may be a single item, thousands of items -> never return null, always list/collection, don't throw exception.
Then getSingleResultOrNull
could be named just findSingleResult
, which would exactly describe what it does for "select
" queries, but there are also SQL procedures, where the result might be just a side effect - and that is probably why we have getResultList
and getSingleResult
. Then getSingleResultOrNull
is fine and useful, but I don't see any advantage in using Optional
.
There are few cases when Optional
helped readability, but too often it simply doesn't bring a value as caller still has to resolve all variants (and sometimes even more).
Btw that throwing exception from getSingleResult()
is quite unpleasant experience for years - usually the caller has some business logic around, using own "exception set", and this just complicated the processing.
find()
vs get()
is a sensible convention indeed.
I understand that there is an opinion that Optional
is an abomination. There are people on the other hand, that consider that the null
itself is an abomination (aka the 1 billion dollar mistake) and that whenever possible, methods should never return nor accept null
s.
Anyway, Optional
is here to stay, some APIs work with it and some people use them. Manually wrapping null
s into Optional
s, besides being easy and trivial is cumbersome and error-prone, just as boxing/unboxing was prior to Java 5.
Even being somewhat deceptive in its capabilities, using Optional
and never returning null
forces the developer to test if a result is present or not, which avoids NullPointerException
s down the stream when someone forget, neglect or misunderstand that a result might not be there (a very common mistake). However, when using an Optional
, failing to handle that case results in a compile error about mismatching types, something that is much harder to forget about or to neglect. In the end of the day, this results in a more trustworthy and less buggy code for API's consumers. And that is the main advantage of the Optional
: Forcing the consumer to test if the result is there or not (without relying on checked exceptions that are plagued with other problems).
So, this would be very convenient in the TypedQuery
interface:
public default Optional<T> getOptionalResult() {
return Optional.ofNullable(getSingleResultOrNull());
}
Anyway, nobody would be forced or obliged to use this, it is just a convenience. People who dislike Optional
could just use getSingleResultOrNull()
instead (or whatever is its name).
I understand that there is an opinion that
Optional
is an abomination. There are people on the other hand, that consider that thenull
itself is an abomination
Well that's a false dichotomy. Both of these things are terrible—untypesafe null
values being the original "abomination", and Optional
being the ugly band-aid non-solution that does little to improve the situation.
Some of us have been advocating for typesafe null
for, oooh, something like twenty years now. (Some of us even designed whole programming languages 🐘 to show that such a construct works really well in practice.) That doesn't mean we're obligated to agree that Optional
was a good idea, or that it makes sense in the context of a given API.
Just fyi, I think that https://openjdk.org/jeps/8316779 will help deal with nulls in all APIs in the future. The discussion to add type restrictions to Java was the last piece missing for us to start null annotating Hibernate ORM. So when this becomes available, we can easily make the switch to the language provided nullability feature. This would IMO be the way forward which also forces people to do something about nullable values.