Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
garydgregory merged PR #1215: URL: https://github.com/apache/commons-lang/pull/1215 -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
garydgregory commented on PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#issuecomment-2137608295 @imbf The latest changes break the build. Run 'mvn' by itself to run all build checks before you push. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
imbf commented on code in PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#discussion_r1618998089 ## src/test/java/org/apache/commons/lang3/function/ConsumersTest.java: ## @@ -46,4 +51,23 @@ public void testNop() { Consumers.nop().accept(""); } +/** + * Tests {@link Consumers#accept(Object, Consumer)}. + */ +@Test +public void testAccept() { +final StringBuilder builder = new StringBuilder("foo"); Review Comment: Thank you for your feedback. I modified my code to not use mock testing. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
garydgregory commented on code in PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#discussion_r1617147931 ## src/test/java/org/apache/commons/lang3/function/ConsumersTest.java: ## @@ -46,4 +51,23 @@ public void testNop() { Consumers.nop().accept(""); } +/** + * Tests {@link Consumers#accept(Object, Consumer)}. + */ +@Test +public void testAccept() { +final StringBuilder builder = new StringBuilder("foo"); Review Comment: Don't use mock testing here, it makes the test too obtuse; this is not a hard feature to test IMO. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
imbf commented on PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#issuecomment-2133582628 @garydgregory Initially, I added apis that only work when the object is null. In this case, there were many useful call sites in this project. (e.g. `ArrayFill.fill()`) After applying https://github.com/apache/commons-lang/pull/1215#discussion_r1611600920, 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? -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
imbf commented on code in PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#discussion_r1615359136 ## src/main/java/org/apache/commons/lang3/function/Functions.java: ## @@ -41,4 +41,20 @@ public static Function function(final Function function) { private Functions() { // no instances needed. } + +/** + * Applies the {@link Function} on the object if the object and the function are not {@code null}. Otherwise, do + * nothing. + * + * @param object the object to apply the function. + * @param function the function to apply. + * @param the type of the argument the function applies. + * @param the type of the result the function returns. + * @return the value the function returns if the object and the function are not {@code null}; {@code null} + * otherwise. + * @since 3.15.0 + */ +public static R applyIfNotNull(final T object, final Function function) { +return object != null && function != null ? function.apply(object) : null; Review Comment: That sounds great. null can also be a valid input. Thank you. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
garydgregory commented on code in PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#discussion_r1611600584 ## src/main/java/org/apache/commons/lang3/function/Consumers.java: ## @@ -46,4 +46,17 @@ private Consumers() { // No instances. } +/** + * Applies the given {@link Consumer} action to the object if the object is not null. If the object is null, it does + * nothing. + * + * @param object the object to be consumed. + * @param consumer the consumer to consume. + * @param the type of the argument the consumer accepts. + */ +public static void acceptIfNotNull(final T object, final Consumer consumer) { Review Comment: 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. ## src/main/java/org/apache/commons/lang3/function/Functions.java: ## @@ -41,4 +41,20 @@ public static Function function(final Function function) { private Functions() { // no instances needed. } + +/** + * Applies the {@link Function} on the object if the object and the function are not {@code null}. Otherwise, do + * nothing. + * + * @param object the object to apply the function. + * @param function the function to apply. + * @param the type of the argument the function applies. + * @param the type of the result the function returns. + * @return the value the function returns if the object and the function are not {@code null}; {@code null} + * otherwise. + * @since 3.15.0 + */ +public static R applyIfNotNull(final T object, final Function function) { +return object != null && function != null ? function.apply(object) : null; Review Comment: 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. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
garydgregory commented on PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#issuecomment-2126031072 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... -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
imbf commented on PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#issuecomment-2125997176 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. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
garydgregory commented on code in PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#discussion_r1598843211 ## src/test/java/org/apache/commons/lang3/function/ConsumersTest.java: ## @@ -46,4 +50,15 @@ public void testNop() { Consumers.nop().accept(""); } +/** + * Tests {@link Consumers#acceptIfNotNull(Object, Consumer)}. + */ +@Test +public void testAcceptIfNotNull() { +StringBuilder builder = new StringBuilder("jay"); +Consumers.acceptIfNotNull(builder, sb -> sb.append("-bae")); +assertEquals("jay-bae", builder.toString()); + +assertDoesNotThrow(() -> Consumers.acceptIfNotNull((String) null, string -> fail())); Review Comment: No need for `assertDoesNotThrow` ## src/main/java/org/apache/commons/lang3/function/Functions.java: ## @@ -41,4 +41,18 @@ public static Function function(final Function function) { private Functions() { // no instances needed. } + +/** + * Applies the {@link Function} on the object if the object is not {@code null}. If the object is {@code null}, it + * does nothing. + * + * @param object the object to apply the function. + * @param function the function to apply. + * @param the type of the argument the function applies. + * @param the type of the result the function returns. + * @return the value the function returns If the object is null; null otherwise. Review Comment: - The name can just be `apply`. - Add a Javadoc since tag. - Add null-check for `function`, see `Suppliers.get()` ## 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}. Review Comment: Unnecessary edit. ## src/main/java/org/apache/commons/lang3/function/Consumers.java: ## @@ -46,4 +46,17 @@ private Consumers() { // No instances. } +/** + * Applies the given {@link Consumer} action to the object if the object is not null. If the object is null, it does + * nothing. + * + * @param object the object to be consumed. + * @param consumer the consumer to consume. + * @param the type of the argument the consumer accepts. + */ +public static void acceptIfNotNull(final T object, final Consumer consumer) { Review Comment: - The name can just be `accept`. - Add a Javadoc since tag. - Add null-check for consumer, see `Suppliers.get()` ## src/test/java/org/apache/commons/lang3/function/ConsumersTest.java: ## @@ -46,4 +50,15 @@ public void testNop() { Consumers.nop().accept(""); } +/** + * Tests {@link Consumers#acceptIfNotNull(Object, Consumer)}. + */ +@Test +public void testAcceptIfNotNull() { +StringBuilder builder = new StringBuilder("jay"); +Consumers.acceptIfNotNull(builder, sb -> sb.append("-bae")); Review Comment: Don't use your name as test data, just use data like foo and bar like you did in the other test. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
imbf commented on PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#issuecomment-2107907901 Thanks for your feedback. I moved null handling features to `Consumers` and `Functions` class in the function package. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
garydgregory commented on PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#issuecomment-2106385760 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. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
jochenw commented on PR #1215: URL: https://github.com/apache/commons-lang/pull/1215#issuecomment-2106378634 @garydgregory Have to admit, that even I don't understand, what you are suggesting? -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] LANG-1733: Add null handling features working with Java 8 specs [commons-lang]
imbf opened a new pull request, #1215: URL: https://github.com/apache/commons-lang/pull/1215 JIRA Ticket: [LANG-1733](https://issues.apache.org/jira/projects/LANG/issues/LANG-1733?filter=allissues) 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. -- 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. To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org