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