gnodet commented on code in PR #12910:
URL: https://github.com/apache/maven/pull/12910#discussion_r3886943404


##########
api/maven-api-xml/src/main/java/org/apache/maven/api/xml/ImmutableCollections.java:
##########
@@ -374,6 +374,23 @@ private ROProperties(Properties props) {
             }
         }
 
+        private static Properties copyDefaults(Properties props) {
+            if (props == null) {
+                return null;
+            }
+            Properties defaults = null;

Review Comment:
   +1 on @elharo's point. The lazy init avoids an allocation when the source 
`Properties` has no defaults chain (the common case), but it trades clarity for 
a marginal optimization. 
   
   A simpler version would be:
   ```java
   private static Properties copyDefaults(Properties props) {
       if (props == null) {
           return null;
       }
       Properties defaults = new Properties();
       for (String name : props.stringPropertyNames()) {
           if (!(props.get(name) instanceof String)) {
               defaults.setProperty(name, props.getProperty(name));
           }
       }
       return defaults.isEmpty() ? null : defaults;
   }
   ```
   
   Or even just always return a (possibly empty) `Properties` and drop the 
ternary — `super(emptyProperties)` is harmless.
   
   That said, the current code is correct as-is — just a readability nit.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to