-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[LANG-1656] Add Supplier-based variants of firstNonBlank and firstNonEmpty for short-circuit evaluation #1469
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Introduced a new method: - StringUtils.firstNonBlankSupplier(Supplier<String>... suppliers) This variant of firstNonBlank accepts suppliers evaluated lazily in order until a non-blank value is found. It enables short-circuit behavior to avoid unnecessary or expensive computations.
Introduced a new method: - StringUtils.firstNonEmptySupplier(Supplier<String>... suppliers) This variant of firstNonEmpty accepts suppliers evaluated lazily in order until a non-empty value is found. It enables short-circuit behavior to avoid unnecessary or expensive computations.
Fixed a minor typo on method name: - StringUtils.firstNonEmptySuppler -> StringUtils.firstNonEmptySupplier
Added unit tests in StringUtilsEmptyBlankTest to verify: - StringUtils.firstNonBlankSupplier() - StringUtils.firstNonEmptySupplier() For each method, test: - Correct return values - null handling - Lazy evaluation / short-circuit behaviour
Fixed Checkstyle issues:
- Local variables are declared 'final'
- Reordered imports (Static imports go first)
- LeftCurly '{' fixes
|
I don't see the need for such a utility. Can this even be reused in Lang? What's a use-case in another code base, like any other Commons component? |
|
Thanks for the feedback. I understand your concerns about reusability. There is actually prior use of this pattern in other Commons components. For example, org.apache.commons.cli.CommandLine in Commons-CLI defines multiple getOptionValue and getParsedOptionValue overloads that accept a Supplier as a lazy default value. This shows that Commons libraries already adopt the Supplier pattern for deferred evaluation. The proposed StringUtils.firstNonBlankSupplier() and firstNonEmptySupplier() generalise this concept: instead of one fallback supplier, they support a sequence of suppliers, evaluated lazily until a non-blank result is found. If the team feels the method is too niche for Lang, another approach is to mark it as “advanced/optional” in the javadoc, or maybe present it as part of a “supplier”-variants section. But I believe its generic nature warrants inclusion. |
Added Generic type T to methods firstNonBlankSupplier() and firstNonEmptySupplier(). - Matches original function - Helps with scalability and backwards compatibility.
|
I'm not convinced that there is a good use case for this. Please can some real-world examples be provided? |
|
A concrete use case for StringUtils.firstNonBlankSupplier() exists in the assignOneCluster method of the SinkServiceImpl class in Apache Inlong. Both assignFromExist() and assignFromRelated() are evaluated eagerly, even though firstNonBlank will only use the first non-blank result. These methods can be non-trivial (e.g., database lookups or network calls). Using the new supplier-based version would avoid that overhead: return StringUtils.firstNonBlankSupplier( |
Fixes to javadoc warnings. - Added params for <T>
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.Description of Issue
The PR addresses the JIRA issue titled 'Short-circuit operation of firstNonBlank and firstNonEmpty', a minor feature request originally proposed by Zhengkai Wang.
The existing methods
firstNonBlankandfirstNonEmptyin StringUtils.java evaluate all input strings immediately. This can lead to extra computational overhead when some arguments are expensive to compute, such as those involving remote API calls or database queries.The feature request aims to introduce supplier-based variants of these methods, which support lazy evaluation and short-circuit behaviour.
Implementation
To resolve this, two new methods were added to StringUtils:
Introducing new methods instead of modifying existing ones ensures backward compatibility with existing firstNonBlank and firstNonEmpty implementations. It avoids method overloading ambiguity and also creates a clear separation between eager and lazy variants.
Validation and Testing
testFirstNonBlankSupplier()andtestFirstNonEmptySupplier()validate:testFirstNonBlankSupplierLazyEvaluation()andtestFirstNonEmptySupplierLazyEvaluation()verify short-circuit behavior using AtomicBoolean flags.