You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/28 21:17:24 UTC

[GitHub] [commons-lang] bindul opened a new pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

bindul opened a new pull request #684:
URL: https://github.com/apache/commons-lang/pull/684


   Utility methods that take a java.util.function.Consumer and possibly null value(s). The consumer is invoked if the value is not null or with the first non-null value, respectively.
   
   See [LANG-1634](https://issues.apache.org/jira/browse/LANG-1634) and discussion at [\[lang\] ObjectUtils enhancement - consumer with non-null value](https://lists.apache.org/thread.html/r5c02039bc54d455f032d06bc196aa7c59d7673a100684272961007e3%40%3Cdev.commons.apache.org%3E)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/36143331/badge)](https://coveralls.io/builds/36143331)
   
   Coverage increased (+0.005%) to 94.986% when pulling **7684dac30487198644269a9979248b80ca0680a1 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **e739844b11c32a88bc04ebb39c9b433d364832cd on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] bindul commented on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
bindul commented on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-755741326


   @garydgregory Done!
   Please let me know if you want me to squash commits into logical groups (I am thinking 1 with the change and the changes for review comments and another where the new method is used elsewhere in lang)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/36026901/badge)](https://coveralls.io/builds/36026901)
   
   Coverage decreased (-0.003%) to 95.01% when pulling **6ae1ac0e859d7cadff14434bbdfe4c865c17ad86 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **f5c86f6d4e8d9023f9e023fb1cd09ef47c8fd1cf on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory commented on a change in pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#discussion_r549756117



##########
File path: src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
##########
@@ -764,4 +791,13 @@ public int compare(final CharSequence o1, final CharSequence o2) {
 
     }
 
+    static final class ApplyIfNonNullBean {
+        private String value;
+        public String getValue() {
+            return value;
+        }
+        public void setValue(String value) {
+            this.value = value;

Review comment:
       You can add a call to `Objects.requireNonNull()` here instead of the blind assignment:
   ```
   this.value = Objects.requireNonNull(value, "value");
   ```
   

##########
File path: src/main/java/org/apache/commons/lang3/ObjectUtils.java
##########
@@ -226,6 +227,62 @@ public static boolean anyNotNull(final Object... values) {
         return firstNonNull(values) != null;
     }
 
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the first {@code non-null} value from
+     * {@code objects}. If all the values are null, the consumer is not invoked.
+     * </p>
+     *
+     * <p>
+     * The caller is responsible for thread-safety and exception handling of consumer.
+     * </p>
+     *
+     * <pre>
+     * ObjectUtils.applyFirstNonNull(bean::setValue, null)                 - setValue not invoked
+     * ObjectUtils.applyFirstNonNull(bean::setValue, null, "abc", "def")   - setValue invoked with "abc"
+     * ObjectUtils.applyFirstNonNull(v -&gt; bean.setValue(v), "abc")      - setValue invoked with "abc"
+     * </pre>
+     *
+     * @param <T> the type of the object
+     * @param objects  the values to test, may be {@code null} or empty
+     * @param consumer the consumer operation to invoke with the first non-null {@code objects}.
+     * @see #firstNonNull(Object...)
+     * @see #applyIfNonNull(Consumer, Object)
+     * @since 3.12
+     */
+    @SafeVarargs
+    public static <T> void applyFirstNonNull(final Consumer<T> consumer, final T... objects) {
+        applyIfNonNull(consumer, firstNonNull(objects));
+    }
+
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the {@code object} if it is
+     * {@code non-null}, otherwise the consumer is not invoked.
+     * </p>
+     *
+     * <p>
+     * The caller is responsible for thread-safety and exception handling of consumer.
+     * </p>
+     *
+     * <pre>
+     * ObjectUtils.applyIfNonNull(bean::setValue, null)             - setValue not invoked
+     * ObjectUtils.applyIfNonNull(bean::setValue, "abc")            - setValue invoked with "abc"
+     * ObjectUtils.applyIfNonNull(v -&gt; bean.setValue(v), "abc")  - setValue invoked with "abc"
+     * </pre>
+     *
+     * @param <T> the type of the object
+     * @param object the {@code Object} to test, may be {@code null}
+     * @param consumer the consumer operation to invoke with {@code object} if it is {@code non-null}
+     * @see #applyFirstNonNull(Consumer, Object...)
+     * @since 3.12
+     */
+    public static <T> void applyIfNonNull(final Consumer<T> consumer, final T object) {

Review comment:
       See "accept" comment above and other comment in the Conversation stream of this PR.
   

##########
File path: src/main/java/org/apache/commons/lang3/ObjectUtils.java
##########
@@ -226,6 +227,62 @@ public static boolean anyNotNull(final Object... values) {
         return firstNonNull(values) != null;
     }
 
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the first {@code non-null} value from

Review comment:
       IMO, you're not "invoking", you're "accepting", so the Javadoc should say "Accepts...". Actually, the method is misnamed since the underlying method is `Consumer#accept()`, this method should be use "accept", not "apply".
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-752129692


   It seems to me the true test of a new API like `applyIfNonNull` is where can it be used within Commons Lang, if we don't eat our own dog food, it seems odd to tell others to do so. IOW, I'd like to see this PR include using this API.
   
   I'm not convinced as to the utility of `applyFirstNonNull`, see above.
   
   Also, for my money, I'd flip the arguments:
   ```
   ObjectUtils.applyIfNonNull(null, bean::setValue);
   ```
   Which reads to me like "if the object is non-null, then apply the function" but then maybe the method should be `ifNonNullApply`, at least that's how my left-to-right reading brain works ;-)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/35996999/badge)](https://coveralls.io/builds/35996999)
   
   Coverage increased (+0.01%) to 95.01% when pulling **d5cdd4e5e1551872df2155c9e0fbc15a71bf4681 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **b5680c55f304ad97d62b50aac531da5f9ae345e2 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/36546890/badge)](https://coveralls.io/builds/36546890)
   
   Coverage increased (+0.005%) to 94.986% when pulling **adf1e04fb5d614ce6417ce166a90246162f8de60 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **6b03fe54b8a434435bea6127048d380e414404b8 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] bindul commented on a change in pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
bindul commented on a change in pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#discussion_r549495161



##########
File path: src/main/java/org/apache/commons/lang3/ObjectUtils.java
##########
@@ -226,6 +227,62 @@ public static boolean anyNotNull(final Object... values) {
         return firstNonNull(values) != null;
     }
 
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the first {@code non-null} value from
+     * {@code objects}. If all the values are null, the consumer is not invoked.
+     * </p>
+     *
+     * <p>
+     * The caller is responsible for thread-safety and exception handling of consumer.
+     * </p>
+     *
+     * <pre>
+     * ObjectUtils.applyIfNonNull(bean::setValue, null)                 - setValue not invoked

Review comment:
       Corrected, thanks.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls commented on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/35975635/badge)](https://coveralls.io/builds/35975635)
   
   Coverage increased (+0.002%) to 95.014% when pulling **ce7e4c98aea3cc2abd9a4029be402416efeecaf7 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **9fa1d734f66f1ad236cafea54bfcc94a1ba8fd1a on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] bindul commented on a change in pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
bindul commented on a change in pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#discussion_r549838166



##########
File path: src/main/java/org/apache/commons/lang3/ObjectUtils.java
##########
@@ -226,6 +227,62 @@ public static boolean anyNotNull(final Object... values) {
         return firstNonNull(values) != null;
     }
 
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the first {@code non-null} value from

Review comment:
       I have changed the method name to 'accept...' and updated the Javadoc comment accordingly.

##########
File path: src/main/java/org/apache/commons/lang3/ObjectUtils.java
##########
@@ -226,6 +227,62 @@ public static boolean anyNotNull(final Object... values) {
         return firstNonNull(values) != null;
     }
 
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the first {@code non-null} value from
+     * {@code objects}. If all the values are null, the consumer is not invoked.
+     * </p>
+     *
+     * <p>
+     * The caller is responsible for thread-safety and exception handling of consumer.
+     * </p>
+     *
+     * <pre>
+     * ObjectUtils.applyFirstNonNull(bean::setValue, null)                 - setValue not invoked
+     * ObjectUtils.applyFirstNonNull(bean::setValue, null, "abc", "def")   - setValue invoked with "abc"
+     * ObjectUtils.applyFirstNonNull(v -&gt; bean.setValue(v), "abc")      - setValue invoked with "abc"
+     * </pre>
+     *
+     * @param <T> the type of the object
+     * @param objects  the values to test, may be {@code null} or empty
+     * @param consumer the consumer operation to invoke with the first non-null {@code objects}.
+     * @see #firstNonNull(Object...)
+     * @see #applyIfNonNull(Consumer, Object)
+     * @since 3.12
+     */
+    @SafeVarargs
+    public static <T> void applyFirstNonNull(final Consumer<T> consumer, final T... objects) {
+        applyIfNonNull(consumer, firstNonNull(objects));
+    }
+
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the {@code object} if it is
+     * {@code non-null}, otherwise the consumer is not invoked.
+     * </p>
+     *
+     * <p>
+     * The caller is responsible for thread-safety and exception handling of consumer.
+     * </p>
+     *
+     * <pre>
+     * ObjectUtils.applyIfNonNull(bean::setValue, null)             - setValue not invoked
+     * ObjectUtils.applyIfNonNull(bean::setValue, "abc")            - setValue invoked with "abc"
+     * ObjectUtils.applyIfNonNull(v -&gt; bean.setValue(v), "abc")  - setValue invoked with "abc"
+     * </pre>
+     *
+     * @param <T> the type of the object
+     * @param object the {@code Object} to test, may be {@code null}
+     * @param consumer the consumer operation to invoke with {@code object} if it is {@code non-null}
+     * @see #applyFirstNonNull(Consumer, Object...)
+     * @since 3.12
+     */
+    public static <T> void applyIfNonNull(final Consumer<T> consumer, final T object) {

Review comment:
       I have changed the method name to 'accept...', switched the parameter order and updated the Javadoc comment accordingly.

##########
File path: src/test/java/org/apache/commons/lang3/ObjectUtilsTest.java
##########
@@ -764,4 +791,13 @@ public int compare(final CharSequence o1, final CharSequence o2) {
 
     }
 
+    static final class ApplyIfNonNullBean {
+        private String value;
+        public String getValue() {
+            return value;
+        }
+        public void setValue(String value) {
+            this.value = value;

Review comment:
       This inner class was meant as a simple Java bean with a setter and getter, hence did not have any checks in it. I have changed it now.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory commented on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-755682248


   @bindul Please rebase on master. TY!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/35976006/badge)](https://coveralls.io/builds/35976006)
   
   Coverage increased (+0.002%) to 95.014% when pulling **e5de898d90df92242200fc6efdbe159b874ca717 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **9fa1d734f66f1ad236cafea54bfcc94a1ba8fd1a on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory commented on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-752129692


   It seems to me the true test of a new API like `applyIfNonNull` is where can it be used within Commons Lang, if we don't eat our own dog food, it seems odd to tell others to do so. IOW, I'd like to see this PR include using this API.
   
   I'm not convinced as to the utility of `applyFirstNonNull`, see above.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/36372615/badge)](https://coveralls.io/builds/36372615)
   
   Coverage decreased (-0.001%) to 94.98% when pulling **2e65abdbaadebe275bca82e6eab22972cfa1bd68 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **c7c74e936db04afeef3af431a597b16f923f6f03 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
garydgregory edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-752129692


   It seems to me the true test of a new API like `applyIfNonNull` is where can it be used within Commons Lang, if we don't eat our own dog food, it seems odd to tell others to do so. IOW, I'd like to see this PR include using this API.
   
   I'm not convinced as to the utility of `applyFirstNonNull`, see above.
   
   Also, for my money, I'd flip the arguments:
   ```
   ObjectUtils.applyIfNonNull(null, bean::setValue);
   ```
   Which reads to me like "if the object is non-null, then apply the function".


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/36169595/badge)](https://coveralls.io/builds/36169595)
   
   Coverage increased (+0.005%) to 94.986% when pulling **1813f187acfcb2300696514d6e6fe4ba30b03414 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **f5ce8c408aa7047efa0f9d418c2d93eddb0a4a63 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] sgflt commented on a change in pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
sgflt commented on a change in pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#discussion_r549491582



##########
File path: src/main/java/org/apache/commons/lang3/ObjectUtils.java
##########
@@ -226,6 +227,62 @@ public static boolean anyNotNull(final Object... values) {
         return firstNonNull(values) != null;
     }
 
+    /**
+     * <p>
+     * Invokes the given {@code consumer's} {@link Consumer#accept(Object)} with the first {@code non-null} value from
+     * {@code objects}. If all the values are null, the consumer is not invoked.
+     * </p>
+     *
+     * <p>
+     * The caller is responsible for thread-safety and exception handling of consumer.
+     * </p>
+     *
+     * <pre>
+     * ObjectUtils.applyIfNonNull(bean::setValue, null)                 - setValue not invoked

Review comment:
       typo in method name




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/36074207/badge)](https://coveralls.io/builds/36074207)
   
   Coverage decreased (-0.003%) to 94.988% when pulling **bde0870e64d4351f558c3377a037e792d35ac9fe on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **3ba01073f647da53e4ee662a491be88728cb8b6c on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-751870250


   
   [![Coverage Status](https://coveralls.io/builds/35993699/badge)](https://coveralls.io/builds/35993699)
   
   Coverage decreased (-0.003%) to 95.01% when pulling **0128401c1b3b9a90fa6d119268f8a38b79e8b702 on bindul:LANG-1634_ObjectUtils-applyIfNonNull** into **cd013c9239b1c9d2f9c910879155daae3f216eef on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] bindul commented on pull request #684: [LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Posted by GitBox <gi...@apache.org>.
bindul commented on pull request #684:
URL: https://github.com/apache/commons-lang/pull/684#issuecomment-752234673


   @garydgregory 
   
   I have renamed the methods to accept from apply and switched the parameter order where possible.
   
   `acceptFirstNonNull` is the analogous method to existing `ObjectUtils.firstNonNull` or `StringUtils.firstNonBlank`. If I remember correctly, I have had one use for this method in my real life code, where a configuration parameter value could be supplied at multiple levels (specific instance, application default or global default; the first two could be null) and I have used a similar method to handle this. If you don't want to include it, I am happy to remove this.
   
   In a real life scenario, the use case for the `acceptIfNonNull` method is usually when setting configuration values. The driver (say for something like redis, solr, etc.) or library (like a HTTP client library) accepts a configuration bean with some default values set. Your application accepts configuration values, and you only want to set those in the configuration bean if they are not null (or you lose the default). Anyways, I have updated 3 existing commons-lang classes to show how it can reduce 3 lines of code to 1. I did not go and update every possible occurrence, as that would make this PR large and lose focus on the primary change in the PR.
   
   Please review and let me know if I should make further modifications.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org