commons-lang icon indicating copy to clipboard operation
commons-lang copied to clipboard

[LANG-1634] Add ObjectUtils #applyIfNonNull and #applyFirstNonNull

Open bindul opened this issue 4 years ago • 6 comments

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 and discussion at [lang] ObjectUtils enhancement - consumer with non-null value

bindul avatar Dec 28 '20 21:12 bindul

Coverage Status

Coverage increased (+0.005%) to 94.986% when pulling adf1e04fb5d614ce6417ce166a90246162f8de60 on bindul:LANG-1634_ObjectUtils-applyIfNonNull into 6b03fe54b8a434435bea6127048d380e414404b8 on apache:master.

coveralls avatar Dec 28 '20 21:12 coveralls

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 ;-)

garydgregory avatar Dec 29 '20 15:12 garydgregory

@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.

bindul avatar Dec 29 '20 20:12 bindul

@bindul Please rebase on master. TY!

garydgregory avatar Jan 06 '21 20:01 garydgregory

@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)

bindul avatar Jan 06 '21 22:01 bindul

We could really use this method. This is helpful because it removes the if/else code branch from the application codes, and therefore, devs don't have to write trivial unit tests to meet the code coverage requirements.

singhbaljit avatar Jan 07 '23 05:01 singhbaljit