Java language migration updates #2020
Conversation
…ement lambdas, method references, avoid unneccessary boxing, use Objects.equals, diamond operator without explicit type where permitted, fix raw usage of parametrized classes)
| @@ -29,7 +30,7 @@ | |||
| public Handler decorateHandler(Handler base, Class<?> sqlObjectType, Method method) { | |||
| final Transaction txnAnnotation = Stream.of(method, sqlObjectType) | |||
| .map(ae -> ae.getAnnotation(Transaction.class)) | |||
| .filter(txn -> txn != null) | |||
| .filter(Objects::nonNull) | |||
There was a problem hiding this comment.
this is one of those places where the explicit code is more readable than the function reference. Now I have to look up what "Objects::nonNull" does. The previous code was more readable.
There was a problem hiding this comment.
it's a matter of taste :)
personally I like method references
| @@ -42,7 +42,7 @@ public ClasspathBuilder setExtension(@Nullable String extension) { | |||
| } | |||
|
|
|||
| // org.foo.Bar$Inner -> org/foo/Bar$Inner | |||
| public ClasspathBuilder appendFullyQualifiedClassName(@Nonnull Class clazz) { | |||
| public ClasspathBuilder appendFullyQualifiedClassName(@Nonnull Class<?> clazz) { | |||
There was a problem hiding this comment.
I think this change is ok, but it does change a signature (from raw type to any type). @stevenschlansker to double check? It is an internal class, though, so it should be fine.
There was a problem hiding this comment.
Of course we can remove this change if you feel unsure.
There was a problem hiding this comment.
Internal APIs are OK to change without regard for compatibility.
|
First, thank you for that PR! Most changes are cosmetic; I appreciate that this makes our code base more idiomatic Java 8+ code. I glanced at the test changes; those all seem good. I wrote a couple of comments on the actual code, again all seems fine. I will try to get the LGTM tests to finish (they crashed halfway through, which seems independent of the actual PR) and merge later today. Did you hand-make those changes or did you use some tool? I know of people that use the modernizer plugin (https://github.com/gaul/modernizer-maven-plugin) but I had very mixed results with it. |
...
Thank you very much for your feedback! This analysis is in addition to compiler warnings, spotbugs, pmd, checkstyle, all of which you have in place in jdbi. |
|
Hi @hgschmie, @stevenschlansker |
|
sorry, got distracted. merged. |
|
Changes look OK to me |
|
Thanks for merging. |
This pull request contains a number of Java language migration updates.
All changes are non-functional. They aim to improve code quality and make use of new Java language features.
Tests are green.