[LANG-1733] Add null-safe Consumers.accept() and Functions.apply() #1215

imbf · 2024-05-12T16:17:30Z

Suggestion: Introduce null handling features working with with the java.util.function package, or more generally, with Java 8 lambdas.

It simplifies null handling by eliminating unnecessary steps such as using specific types or if/else statements.
It reduces the overhead associated with managing objects like the Optional type.
It decreases the complexity involved in other null handling methods.

garydgregory

@imbf
Please follow the pattern established by the 'Suppliers' class in this package. There is no need for a new class. TY.

jochenw · 2024-05-12T21:24:29Z

@garydgregory Have to admit, that even I don't understand, what you are suggesting?

codecov-commenter · 2024-05-12T21:26:22Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.54%. Comparing base (f962e7b) to head (f2c71db).
Report is 219 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1215      +/-   ##
============================================
+ Coverage     92.07%   92.54%   +0.46%     
- Complexity     7587     7757     +170     
============================================
  Files           200      200              
  Lines         15844    15901      +57     
  Branches       2890     2819      -71     
============================================
+ Hits          14589    14715     +126     
+ Misses          682      648      -34     
+ Partials        573      538      -35

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

garydgregory · 2024-05-12T21:53:51Z

Our Suppliers class implements a null-safe get method. If you want to do a null-safe version of accept for Consumers, then that new method should go in our Consumers class, not a new class.

imbf · 2024-05-13T15:05:50Z

Thanks for your feedback.

I moved null handling features to Consumers and Functions class in the function package.

garydgregory

@imbf
Thank you for your update. Please see my comments.

garydgregory · 2024-05-13T17:49:53Z

src/main/java/org/apache/commons/lang3/function/Consumers.java

+     * @param consumer the consumer to consume.
+     * @param <T> the type of the argument the consumer accepts.
+     */
+    public static <T> void acceptIfNotNull(final T object, final Consumer<T> consumer) {


The name can just be accept.

Add a Javadoc since tag.

Add null-check for consumer, see Suppliers.get()

I disagree that the API is noop if the input to the functional object is null. A lot of APIs accept null as valid input, for example to reset a value to its default.

garydgregory · 2024-05-13T17:51:42Z

src/main/java/org/apache/commons/lang3/function/Consumers.java

@@ -21,7 +21,7 @@
 import java.util.function.Function;

 /**
- * Provides {@link Consumer} instances.
+ * Provides {@link Consumer} instances and utilities for working with {@link Consumer}.


Unnecessary edit.

garydgregory · 2024-05-13T17:52:25Z

src/main/java/org/apache/commons/lang3/function/Functions.java

+     * @param function the function to apply.
+     * @param <T> the type of the argument the function applies.
+     * @param <R> the type of the result the function returns.
+     * @return the value the function returns If the object is null; null otherwise.


The name can just be apply.

Add a Javadoc since tag.

Add null-check for function, see Suppliers.get()

garydgregory · 2024-05-13T17:53:52Z

src/test/java/org/apache/commons/lang3/function/ConsumersTest.java

+        Consumers.acceptIfNotNull(builder, sb -> sb.append("-bae"));
+        assertEquals("jay-bae", builder.toString());
+
+        assertDoesNotThrow(() -> Consumers.acceptIfNotNull((String) null, string -> fail()));


No need for assertDoesNotThrow

garydgregory · 2024-05-13T17:54:58Z

src/test/java/org/apache/commons/lang3/function/ConsumersTest.java

+    @Test
+    public void testAcceptIfNotNull() {
+        StringBuilder builder = new StringBuilder("jay");
+        Consumers.acceptIfNotNull(builder, sb -> sb.append("-bae"));


Don't use your name as test data, just use data like foo and bar like you did in the other test.

imbf · 2024-05-23T00:44:41Z

Thank you for your feedback.

I applied most of feedbacks. but the name suggestion is not good for me.
I think your suggestion is short and easy but the existing name describes the functionality of the method more accurately.

garydgregory · 2024-05-23T01:31:13Z

Well, you can change the names now, and save me the work, or I'll change them after I review the PR and it is merged...

garydgregory

@imbf
Please see my additional comments.
Are there call sites within Commons Lang where it would be beneficial to use these new APIs? If the potential call sites are already null-safe concerning the functional objects, then those would not be candidates, but, there may be others.

garydgregory · 2024-05-23T12:24:32Z

src/main/java/org/apache/commons/lang3/function/Consumers.java

+     * @param consumer the consumer to consume.
+     * @param <T> the type of the argument the consumer accepts.
+     */
+    public static <T> void acceptIfNotNull(final T object, final Consumer<T> consumer) {


I disagree that the API is noop if the input to the functional object is null. A lot of APIs accept null as valid input, for example to reset a value to its default.

garydgregory · 2024-05-23T12:24:50Z

src/main/java/org/apache/commons/lang3/function/Functions.java

+     * @since 3.15.0
+     */
+    public static <T, R> R applyIfNotNull(final T object, final Function<T, R> function) {
+        return object != null && function != null ? function.apply(object) : null;


I disagree that the API is noop if the input to the functional object is null. A lot of APIs accept null as valid input, for example to reset a value to its default.

That sounds great. null can also be a valid input.

Thank you.

… null

imbf · 2024-05-27T14:19:19Z

@garydgregory

Initially, I added apis that only work when the object is not null. In this case, there were many useful call sites in this project. (e.g. ArrayFill.fill()) After applying #1215 (comment), these apis accepted null as valid input and lost null-safe functionality.

This is not my intent. If you want to apply a functional interface regardless of whether the object is null or not, making other methods is better than fixing existing methods.

What do you think about my opinion?

garydgregory

Hello @imbf
Thank you for your update.
I added one comment.

garydgregory · 2024-05-28T12:21:09Z

src/test/java/org/apache/commons/lang3/function/ConsumersTest.java

+     */
+    @Test
+    public void testAccept() {
+        final StringBuilder builder = new StringBuilder("foo");


Don't use mock testing here, it makes the test too obtuse; this is not a hard feature to test IMO.

Thank you for your feedback.
I modified my code to not use mock testing.

garydgregory · 2024-05-29T14:49:54Z

@imbf
The latest changes break the build.
Run 'mvn' by itself to run all build checks before you push.

LANG-1733: Add null handling features working with Java 8 specs

547eded

garydgregory requested changes May 12, 2024

View reviewed changes

LANG-1733: Move null handling features to Consumers and Functions

1a8a21e

LANG-1733: Update Javadoc

463d9d3

garydgregory requested changes May 13, 2024

View reviewed changes

배종진 added 3 commits May 23, 2024 09:41

LANG-1733: Add since javadoc tag

c6f3b32

LANG-1733: Add null check for consumer and function

5aac6ba

LANG-1733: Revert Functions and Consumers class javadoc

2b689da

imbf force-pushed the LANG-1733-null-handling-features branch from d419f9b to 2b689da Compare May 23, 2024 00:41

garydgregory requested changes May 23, 2024

View reviewed changes

배종진 added 2 commits May 26, 2024 23:48

LANG-1733: Rename methods

56e77e7

LANG-1733: Update null handling features to handle an object if it is…

42f32e7

… null

garydgregory requested changes May 28, 2024

View reviewed changes

배종진 added 2 commits May 29, 2024 23:24

LANG-1733: Update ConsumersTest to not use mock

e66d428

LANG-1733: Update Comments

50d010b

배종진 added 2 commits May 30, 2024 00:27

LANG-1733: Update codes to pass style check

b14e592

LANG-1733: Modify the comment on apply method in Functions class

f2c71db

garydgregory merged commit 234c6a9 into apache:master May 29, 2024
18 checks passed

garydgregory changed the title ~~LANG-1733: Add null handling features working with Java 8 specs~~ [LANG-1733] Add null-safe Consumers.accept() and Functions.apply() May 29, 2024

garydgregory added a commit that referenced this pull request May 29, 2024

Add null-safe Consumers.accept() and Functions.apply() #1215

4b99af5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LANG-1733] Add null-safe Consumers.accept() and Functions.apply() #1215

[LANG-1733] Add null-safe Consumers.accept() and Functions.apply() #1215

imbf commented May 12, 2024

garydgregory left a comment

jochenw commented May 12, 2024

codecov-commenter commented May 12, 2024 •

edited

garydgregory commented May 12, 2024

imbf commented May 13, 2024

garydgregory left a comment

garydgregory May 13, 2024

garydgregory May 23, 2024

garydgregory May 13, 2024

garydgregory May 13, 2024

garydgregory May 13, 2024

garydgregory May 13, 2024

imbf commented May 23, 2024

garydgregory commented May 23, 2024

garydgregory left a comment

garydgregory May 23, 2024

garydgregory May 23, 2024

imbf May 26, 2024

imbf commented May 27, 2024 •

edited

garydgregory left a comment

garydgregory May 28, 2024

imbf May 29, 2024

garydgregory commented May 29, 2024

[LANG-1733] Add null-safe Consumers.accept() and Functions.apply() #1215

[LANG-1733] Add null-safe Consumers.accept() and Functions.apply() #1215

Conversation

imbf commented May 12, 2024

garydgregory left a comment

Choose a reason for hiding this comment

jochenw commented May 12, 2024

codecov-commenter commented May 12, 2024 • edited

Codecov Report

garydgregory commented May 12, 2024

imbf commented May 13, 2024

garydgregory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imbf commented May 23, 2024

garydgregory commented May 23, 2024

garydgregory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

imbf commented May 27, 2024 • edited

garydgregory left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

garydgregory commented May 29, 2024

codecov-commenter commented May 12, 2024 •

edited

imbf commented May 27, 2024 •

edited