Closed sleberknight closed 2 months ago
Never mind, changing the return type is not binary compatible regardless.
Change result type (including void) --> Breaks compatibility
Also:
Changing the result type of a method, or replacing a result type with void, or replacing void with a result type, has the combined effect of deleting the old method and adding a new method with the new result type or newly void result
source: https://docs.oracle.com/javase/specs/jls/se17/html/jls-13.html#jls-13.4.15
This means we must add new methods if we don't want to go to a new major version. And since we have many Dropwizard services that definitely use the void KiwiResources.verifyExistence(T obj)
methods, we don't want any of these to break.
So, the question becomes: what should the new methods be named? I asked "a friend" (ChatGPT 4o) what "it" thought and got the following:
verifyAndReturn
checkAndReturn
validateAndReturn
verifyPresence
ensureExistence
I said the names must contain "verify" and "existence" and got:
verifyExistenceAndReturn
verifyAndReturnExistence
returnVerifiedExistence
ensureAndReturnExistence
validateExistenceAndReturn
I then told it that 3, 4, and 5 did not contain both "verify" existence" and got an "apology" and these suggestions:
verifyExistenceAndReturn
verifyAndReturnExistence
verifyExistenceWithReturn
verifyExistenceAndProvide
verifyExistenceReturning
The only thing I came up with before asking ChatGPT was verifyExistenceAndReturn
, but I also think verifyExistenceWithReturn
and verifyExistenceReturning
would be somewhat acceptable.
Overall I think verifyExistenceAndReturn
is probably the simplest and most descriptive name for the new methods.
+1: verifyExistenceAndReturn
KiwiResources
has a collection of overloaded methods namedverifyExistence
which have different signatures. Half of them acceptT resourceEntity
and the other half acceptOptional<T> resourceEntity.
All of them throw a
JaxrsNotFoundException
when the object does not exist, either becauseresourceEntity
isnull
or is an emptyOptional
.The methods that accept
Optional<T>
return a (non-null)T
, which allows code like:But, if you have a possibly nullable object, you must write this code:
The above code is safe, but code analysis tools may still report a possible NPE from
latest.getId()
, because they don't understand thatKiwiResources.verifyExistence
will either return a (non-null) object or throw an exception. So then you have to either suppress (e.g., using an annotation, or in Sonar's web UI, etc.) the warning, or wraplatest
withObjects.requireNonNull
even though it can't ever be null if verifyExistence returns without throwing (or just ignore it).This issue addresses the method-chaining by introducing new methods similar to the
void
-returningverifyExistence
methods, or possibly just modifying the existing methods to return the same object when it is not null. I think this should be binary compatible since no existing code can possibly have a return type and compile.A second issue will be created to annotate all the
verifyExistence
methods with an annotation to indicate that they should not ever returnnull
. This will make static analyzers happy, or should anyway.