Speaking of collections... In some code bases I review (not here) a List is often used when a Set would semantically correct. Is there an opportunity in m3 or m4 for such a change?
Gary On Mon, Sep 23, 2024, 6:32 AM Martin Desruisseaux < martin.desruisse...@geomatys.com> wrote: > Hello again > > In my attempt to add a List<Source> to Maven, I got hesitations about > what to do with the following file: > > * > https://github.com/apache/maven/blob/master/maven-core/src/main/java/org/apache/maven/project/MavenProject.java > > This class contains many collections, but I'm not sure if they are all > handled consistently? > > * Some collections are initialized at construction time (e.g. > compileSourceRoots, activeProfiles), other are not (e.g. > dependencyArtifacts). > * Some collections are protected against user's change by wrapping > them in unmodifiable collection (e.g. getBuildPlugins()), other are > not (e.g. getArtifacts()). The latter looks strange because allowing > users to modify the map content can make the artifacts map > inconsistent with other MavenProject states such as getArtifactMap(). > * Most collections are cloned in the deepCopy(MavenProject) method, > but not all of them (e.g. injectedProfileIds, projectReferences). > For example, a call to setInjectedProfileIds(String source, > List<String>) will modify both the instance on which the method is > invoked and any cloned MavenProject instances. > > Would it be okay if I modify the implementation with the following goals? > > * All setter methods make a defensive copy of the given collection, > preserving Set order. > * All getter methods returns an unmodifiable (but not necessarily > immutable) collection. > * The collections of properties that do not have an addFoo(...) method > are immutable. > * The clone method copies all non-immutable collections, and only them. > * MavenProject(MavenProject) constructor is either deprecated, or > modified for assigning fields directly without invoking setter > methods (for avoiding the "this-escaped" compiler warning). > > I did not verified the whole Maven code base, but at least for the parts > that I looked, I saw no code attempting to modify the collections > returned by the getter methods (except MavenProject itself, but it can > easily be fixed). > > Martin > > >
