[LANG-1784] Functions methods for null-safe mapping and chaining#1435
[LANG-1784] Functions methods for null-safe mapping and chaining#1435richdougherty wants to merge 1 commit intoapache:masterfrom
Conversation
These new methods add support for chaining calls to potentially null values, e.g. applyIfNull(person.getName(), String::trim).
78d8574 to
8601577
Compare
| final T value, | ||
| final Function<? super T, ? extends U> mapper1, | ||
| final Function<? super U, ? extends V> mapper2, | ||
| final Function<? super V, ? extends R> mapper3) { |
There was a problem hiding this comment.
Self-review: I'm using some fancy types here - I might add unit tests to check a few type compilation scenarios for this method and the other methods as well.
| */ | ||
| public static <T, R> R applyIfNotNull( | ||
| final T value, | ||
| final Function<? super T, ? extends R> mapper) { |
There was a problem hiding this comment.
Note that the signature and types follow Optional.map.
| * @since 3.19.0 | ||
| */ | ||
| public static <T, R> R applyIfNotNull( | ||
| final T value, |
There was a problem hiding this comment.
I didn't add @Nullable/@Nonnull for these methods because the annotations are not present in other methods in this class, but let me know if you want them. It might need some changes in the project build dependencies though?
| assertFalse(ObjectUtils.anyNull(FOO, BAR, 1, Boolean.TRUE, new Object(), new Object[]{})); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Self-review: I'll add Javadoc like in the other methods in this class.
|
Hello @richdougherty Thank you for your PR. I've considered a single method like this the past but I've never gone ahead and implemented it. Probably because it feels like a bit of a hack for working around the fact that Java has not implemented an Elvis operator. I'm sure there's a YT about that. That said I would simply call the method "apply" and the leave non-null semantics as Javadoc. Why 3 overloads? As soon as there are n overloads, someone will submit a request for n+1. I'd just go with a vararg and be done with it. There are edge cases that could be surprising: What if one of the given functions throws an NPE? Should that cause 'apply' to return null? It might be surprising if a method designed to handle nulls throws an NPE. What if I want to call a method that throws a checked exception? I guess we'd need a FailableFunction version or only have a FailableFunction version (since you can default it to a runtime exception). |
|
Is the AI you used compatible with the Apache License? If you don't know, then a clean room implementation is the way to go. Why in the world do you need AI for such simple code btw? |
|
Thanks for the quick feedback!
Sure, happy to fit into the pattern of the class and name it
OK I see your point. How about I go with a single-function version only, so there are no overloads? We can always chain like follows, which is type-safe and avoid multiple overloads. I did consider varargs, but the issue is that the type of the input, intermediate and return values can differ, and we can't represent that safely in an array of functions. All the functions would need to have the same time - or no type! It would be easy for users to make mistakes and get runtime errors that could otherwise be avoided. So I think a single overload is cleaner if that works.
My preference is to throw any exceptions that the function throws. Perhaps that is surprising though - I can document it, especially that NPEs can be thrown.. I think the method has a simple clear behaviour - it handles null-ness of the By having a more minimal behaviour it satisfies those who want a very basic behaviour, but it's still possible to combine the method with others to get more complex behaviour, e.g. make an
I did create FailableFunction versions at first, but then removed them to simplify, with so many methods. But if we're going with a single non-Failable How does
Just to be clear I wrote the (very simple) logic myself - I definitely didn't need AI to do that! However, I'm declaring AI usage just to be upfront, because I just used it as a peer review to check for any typos, inconsistencies etc. e.g. "can you please review this code and identify any errors or inconsistencies in comments or code". It had a couple of suggestions, which I fixed, e.g. repeating mapper2 in one place, instead of mapper3. (I wrote the fixes) To be safe I have double checked the terms of usage for the AI. No ownership is claimed, and it's not copyright, since the code is original and not what the AI suggested. Again - thanks for the quick feedback. Let me know if I should go ahead with these suggestions:
I think even this simple |
|
Hello @richdougherty Thanks for the detailed reply. Looking over what we have in the library, it now looks like we should not put this in
Now, we'd add a Note that the I don't think the Javadoc should document "alternatives"; that's confusing. It might be I wonder how long it will be until someone asks about a |
|
Great - I'll go ahead and implement that. Thanks for the clear direction. |
chaining #1435 Use the same naming style of Objects.requireNonNull()
|
Hello @richdougherty This is now in git master in the classes Note that the methods are named TY! |
chaining #1435 Use the same naming style of Objects.requireNonNull()
|
Hi @garydgregory , thanks so much. This definitely addresses the common use case I have in mind. Apologies about the time taken - I did have a fix partway through but appreciate you getting it done. |
This PR contains several new proposed methods to add to
ObjectUtilsthat support chaining functions for objects which might be null.The Problem
It's a very common pattern in Java to need to traverse a chain of method calls, where any link in the chain might be null. For example,
a.getB().getC().getD().Handling the null checks can be verbose. Other languages have built-in support for this, like Kotlin's safe navigation operator (
?.), which allows for a concisea?.getB()?.getC()?.getD().(See: https://en.wikipedia.org/wiki/Safe_navigation_operator)
In Java, the two most common approaches are nested conditionals or using
Optional.Using conditionals:
Using
Optional:Proposed solution
The proposed solution is to
The basic idea is to add methods, e.g.
ObjectUtils.applyIfNotNull(...), with overloads for one, two, or three functions.Example with new method:
The methods check for null at each step of the function chain. If any value is null (either the initial input or the result of an intermediate function), the chain is short-circuited and null is returned immediately.
Discussion
mapNonNullorchain, but this is what I settled on.FailableFunctionversions as well, but in the end removed them to keep things simple to start with.Checklist
Thanks for your contribution to Apache Commons! Your help is appreciated!
Before you push a pull request, review this list:
mvn; that'smvnon the command line by itself.Comments
richdougherty@apache.org. Or you can see me on the contributor list here: https://commons.apache.org/proper/commons-collections/team.html