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


##########
src/mdo/model.vm:
##########
@@ -145,55 +145,39 @@ public class ${class.name}
     #end
 
     /**
-      * Constructor for this class, package protected.
+      * Constructor for this class, to be called from its subclasses and 
{@link Builder}.
       * @see Builder#build()
       */
-    ${class.name}(
-    #if ( $class == $root )
-        String namespaceUri,
-        String modelEncoding,
-    #end
-    #foreach ( $field in $allFields )
-      #set ( $sep = 
"#if(${locationTracking}||$field!=${allFields[${allFields.size()} - 1]}),#end" )
-      #set ( $type = 
${types.getOrDefault($field,${types.getOrDefault($field.type,$field.type)})} )
-      #if ( $type.startsWith("List<") )
-        #set ( $type = ${type.replace('List<','Collection<')} )
-      #end
-        $type $field.name${sep}
-    #end
-    #if ( $locationTracking )
-        Map<Object, InputLocation> locations,
-        InputLocation importedFrom
-    #end
-    ) {
+    protected ${class.name}(Builder builder) {
     #if ( $class.superClass )
-        super(
-      #foreach ( $field in $inheritedFields )
-        #set ( $sep = 
"#if(${locationTracking}||$field!=${inheritedFields[${inheritedFields.size()} - 
1]}),#end" )
-            ${field.name}${sep}
-      #end
-      #if ( $locationTracking )
-            locations,
-            importedFrom
-      #end
-        );
+        super(builder);
     #end
     #if ( $class == $root )
-        this.namespaceUri = namespaceUri;
-        this.modelEncoding = modelEncoding;
+        this.namespaceUri = builder.namespaceUri != null ? 
builder.namespaceUri : (builder.base != null ? builder.base.namespaceUri : 
null);
+        this.modelEncoding = builder.modelEncoding != null ? 
builder.modelEncoding : (builder.base != null ? builder.base.modelEncoding :  
"UTF-8");
     #end
     #foreach ( $field in $class.getFields($version) )
       #if ( $field.type == "java.util.List" || $field.type == 
"java.util.Properties" || $field.type == "java.util.Map" )
-        this.${field.name} = ImmutableCollections.copy(${field.name});
+        this.${field.name} = ImmutableCollections.copy(builder.${field.name} 
!= null ? builder.${field.name} : (builder.base != null ? 
builder.base.${field.name} : null));
       #else
-        this.${field.name} = ${field.name};
+        #if ( $field.type == "boolean" || $field.type == "int" )
+        this.${field.name} = builder.${field.name} != null ? 
builder.${field.name} : (builder.base != null ? builder.base.${field.name} : 
${field.defaultValue});
+        #else
+        this.${field.name} = builder.${field.name} != null ? 
builder.${field.name} : (builder.base != null ? builder.base.${field.name} : 
null);
+        #end
       #end
     #end
     #if ( $locationTracking )
+        Map<Object, InputLocation> newlocs = builder.locations != null ? 
builder.locations : Collections.emptyMap();
+        Map<Object, InputLocation> oldlocs = builder.base != null && 
builder.base.locations != null ? builder.base.locations : 
Collections.emptyMap();
       #if ( ! $class.superClass )
-        this.locations = ImmutableCollections.copy(locations);
-        this.importedFrom = importedFrom;
+        this.locations = new HashMap<>();

Review Comment:
   I really dislike the knowledge that is being spread across the 
`Builder#build()` method and the actual constructor.  But the shallow-copy 
would be really a performance loss during the initial transformation of the 
model.
   
   I think it would be much easier to simply change the visibility of 
constructors to `protected`, I think this would be enough to allow extension 
from outside the package, as the generated code already allows extension (as we 
have `Model` extending `ModelBase` and other similar classes).



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

Reply via email to