[GitHub] [maven] gnodet commented on a diff in pull request #816: Small optimisations and cleanup

[GitBox]

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


##########
maven-model-builder/src/main/java/org/apache/maven/model/profile/DefaultProfileActivationContext.java:
##########
@@ -194,15 +162,30 @@ public Map<String, String> getProjectProperties() {
     }
 
     public DefaultProfileActivationContext setProjectProperties(Properties 
projectProperties) {
-        if (projectProperties != null) {
-            this.projectProperties = projectProperties.entrySet().stream()
-                    .collect(collectingAndThen(
-                            toMap(k -> String.valueOf(k.getKey()), v -> 
String.valueOf(v)),
-                            Collections::unmodifiableMap));
-        } else {
-            this.projectProperties = Collections.emptyMap();
-        }
+        return setProjectProperties(toMap(projectProperties));
+    }
+
+    public DefaultProfileActivationContext setProjectProperties(Map<String, 
String> projectProperties) {
+        this.projectProperties = unmodifiable(projectProperties);

Review Comment:
   > Previously, a `null` for the `projectProperties` argument would result in 
`this.projectProperties` being an empty `Map`. With this patch, it would result 
in `this.projectProperties` being `null`. Are you sure that's an intended 
change?
   
   I don't think that's true.  `toMap` can return `null`, but `unmodifiable(x)` 
returns an empty list/map, so `this.projectProperties` can never be `null` 
afaik.



-- 
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...@maven.apache.org

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