You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by "HarshMehta112 (via GitHub)" <gi...@apache.org> on 2025/11/22 07:10:24 UTC

[I] [Improvement] Use orElseGet() Instead of orElse() for Lazy Evaluation [shardingsphere]

HarshMehta112 opened a new issue, #37159:
URL: https://github.com/apache/shardingsphere/issues/37159

   ### What would you like to be improved?
   
   - orElse(...) eagerly evaluates its argument, meaning the fallback expression is computed even when the Optional contains a value.
   - Causing DefaultYamlTupleProcessor to be created even when a suitable ShardingSphereYamlTupleProcessor is found.
   
   - Using orElseGet(...) (or the or(...) API) avoids this by evaluating the fallback only when needed, making the code more efficient and idiomatic.
   
   https://github.com/apache/shardingsphere/blob/555c858f8a307169f37c85b932a02fc80bbdec25/infra/util/src/main/java/org/apache/shardingsphere/infra/util/yaml/representer/ShardingSphereYamlRepresenter.java#L54
   
   ### How should we improve?
   Suggested improvement (lazy evaluation)
   ```
   @Override
   protected NodeTuple representJavaBeanProperty(
           final Object javaBean, final Property property, final Object propertyValue, final Tag customTag) {
       
       NodeTuple nodeTuple = super.representJavaBeanProperty(javaBean, property, propertyValue, customTag);
       return TypedSPILoader.findService(ShardingSphereYamlTupleProcessor.class, property.getName())
               .map(processor -> processor.process(nodeTuple))
               .orElseGet(() -> new DefaultYamlTupleProcessor().process(nodeTuple));
   }
   ```


-- 
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: notifications-unsubscribe@shardingsphere.apache.org.apache.org

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


Re: [I] [Improvement] Use orElseGet() Instead of orElse() for Lazy Evaluation [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu commented on issue #37159:
URL: https://github.com/apache/shardingsphere/issues/37159#issuecomment-3582498723

   ##  Problem Understanding
   
     - Location: infra/util/src/main/java/org/apache/shardingsphere/infra/util/yaml/representer/ShardingSphereYamlRepresenter#representJavaBeanProperty currently uses .orElse(new DefaultYamlTupleProcessor().process(nodeTuple)); proposal is to switch to lazy orElseGet.
     - Goal: avoid creating and executing the default processor when a ShardingSphereYamlTupleProcessor is already found.
   
   ##  Root Cause
   
     - Optional.orElse eagerly evaluates its argument, so the default processor is instantiated and called every time. orElseGet only runs the supplier when the Optional is empty (per Java docs: https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#orElseGet-
       java.util.function.Supplier-).
   
   ##  Analysis
   
     - Behavior: the default processor just filters null/empty nodes and is side‑effect free; eager execution does not change output.
     - Performance: only when many properties hit a custom processor does orElseGet save a tiny object creation and call; if most use the default processor, cost is identical. YAML conversion is not a hotspot, so benefit is negligible.
   
   ##  Conclusion
   
     - This is a semantic/style micro-optimization, not a functional bug. Using orElseGet aligns with lazy semantics; performance gain is minimal but acceptable as a small improvement.
     - Suggested change:
   
   ```java
     return TypedSPILoader.findService(ShardingSphereYamlTupleProcessor.class, property.getName())
             .map(processor -> processor.process(nodeTuple))
             .orElseGet(() -> new DefaultYamlTupleProcessor().process(nodeTuple));
   ```
   
     We’re open to this tweak. Community contributors are warmly invited to submit a PR with the change and corresponding tests—thanks for helping improve ShardingSphere!


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


Re: [I] [Improvement] Use orElseGet() Instead of orElse() for Lazy Evaluation [shardingsphere]

Posted by "terrymanu (via GitHub)" <gi...@apache.org>.
terrymanu closed issue #37159: [Improvement] Use orElseGet() Instead of orElse() for Lazy Evaluation
URL: https://github.com/apache/shardingsphere/issues/37159


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


Re: [I] [Improvement] Use orElseGet() Instead of orElse() for Lazy Evaluation [shardingsphere]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on issue #37159:
URL: https://github.com/apache/shardingsphere/issues/37159#issuecomment-3629392868

   There hasn't been any activity on this issue recently, and in order to prioritize active issues, it will be marked as stale.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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