On 6/19/07, Gilles Scokart <[EMAIL PROTECTED]> wrote:
>
> Please, have a look at this change (Mostly to Ivy).
>
> I did this change in order to eleminate the dependency between the
> SortEngine and the Settings. Now, the Ivy class has responsability to
> inject the settings into the engine.
>
> This eliminate the dependency, clarify the engine, and clarify the
> test a little bit. Howerver the side effect is that the setter
> 'setSortEngine' doesn't work like before. Previously, when the
> setSortEngine method was used, the settings was not injected in the
> engine. Now it is.
>
> WDYT? Do you think the same apoach could be taken for the other
> engines or do you think we should roll back?
The problem with this approach IMO is that it doesn't scale very well. If
sort engine requires a lot of settings, setting properties one by one will
be cumbersome. A solution in between would be to define a Settings interface
for each specific settings. Then we could keep only class implementing all
interfaces, or have one central setting giving access to each
implementation, or rely on a mock object for tests, ... That's the approach
used by Wicket:
https://svn.apache.org/repos/asf/incubator/wicket/trunk/jdk-1.4/wicket/src/main/java/org/apache/wicket/settings/Settings.java
Another point concerning your change: you set the properties in the getter
which is public, meaning that the setters are called each time get is
called, which is too much IMO.
Xavier
Gilles
>
> 2007/6/19, [EMAIL PROTECTED] <[EMAIL PROTECTED]>:
> > Author: gscokart
> > Date: Tue Jun 19 04:32:43 2007
> > New Revision: 548695
> >
> > URL: http://svn.apache.org/viewvc?view=rev&rev=548695
> > Log:
> > refactor: remove dependency between sort engine and IvySettings
> >
> > Modified:
> > incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> >
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> >
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> >
> > Modified: incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java
> > URL:
>
http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java?view=diff&rev=548695&r1=548694&r2=548695
> >
> ==============================================================================
> > --- incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java (original)
> > +++ incubator/ivy/core/trunk/src/java/org/apache/ivy/Ivy.java Tue Jun 19
> 04:32:43 2007
> > @@ -159,7 +159,8 @@
> > eventManager = new EventManager();
> > }
> > if (sortEngine == null) {
> > - sortEngine = new SortEngine(settings);
> > + sortEngine = new SortEngine();
> > + //Settings element are injected in the getSortEngine
> method.
> > }
> > if (searchEngine == null) {
> > searchEngine = new SearchEngine(settings);
> > @@ -329,7 +330,7 @@
> > * Sorts the collection of IvyNode from the less dependent to the
> more dependent
> > */
> > public List sortNodes(Collection nodes) {
> > - return sortEngine.sortNodes(nodes);
> > + return getSortEngine().sortNodes(nodes);
> > }
> >
> > /**
> > @@ -344,15 +345,10 @@
> > * revision of an other modules present in the of
> modules to sort with a different
> > * revision.
> > * @return a List of sorted ModuleDescriptors
> > - * @deprecated Use
> sortModuleDescriptors(Collection,NonMatchingVersionReporter)
> > */
> > - public List sortModuleDescriptors(Collection moduleDescriptors) {
> > - return sortEngine.sortModuleDescriptors(moduleDescriptors, new
> SilentNonMatchingVersionReporter());
> > - }
> > -
> > public List sortModuleDescriptors(Collection moduleDescriptors,
> > NonMatchingVersionReporter nonMatchingVersionReporter) {
> > - return sortEngine.sortModuleDescriptors(moduleDescriptors,
> nonMatchingVersionReporter);
> > + return getSortEngine().sortModuleDescriptors(moduleDescriptors,
> nonMatchingVersionReporter);
> > }
> >
> > //
> ///////////////////////////////////////////////////////////////////////
> > @@ -569,6 +565,8 @@
> > }
> >
> > public SortEngine getSortEngine() {
> > + sortEngine.setCircularDependencyStrategy(
> settings.getCircularDependencyStrategy());
> > + sortEngine.setVersionMatcher(settings.getVersionMatcher());
> > return sortEngine;
> > }
> >
> >
> > Modified:
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> > URL:
>
http://svn.apache.org/viewvc/incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java?view=diff&rev=548695&r1=548694&r2=548695
> >
> ==============================================================================
> > ---
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> (original)
> > +++
> incubator/ivy/core/trunk/src/java/org/apache/ivy/core/sort/SortEngine.java
> Tue Jun 19 04:32:43 2007
> > @@ -26,18 +26,29 @@
> >
> > import org.apache.ivy.core.module.descriptor.ModuleDescriptor;
> > import org.apache.ivy.core.resolve.IvyNode;
> > -import org.apache.ivy.core.settings.IvySettings;
> > import org.apache.ivy.plugins.circular.CircularDependencyException;
> > import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> > import org.apache.ivy.plugins.version.VersionMatcher;
> >
> > public class SortEngine {
> > - private IvySettings settings;
> >
> > - public SortEngine(IvySettings settings) {
> > - this.settings = settings;
> > + private CircularDependencyStrategy circularStrategy;
> > +
> > + private VersionMatcher versionMatcher;
> > +
> > + public SortEngine() {
> > + }
> > +
> > +
> > + public void
> setCircularDependencyStrategy(CircularDependencyStrategy circularStrategy) {
> > + this.circularStrategy = circularStrategy;
> > }
> >
> > + public void setVersionMatcher(VersionMatcher versionMatcher) {
> > + this.versionMatcher = versionMatcher;
> > + }
> > +
> > +
> > public List sortNodes(Collection nodes) throws
> CircularDependencyException {
> > /*
> > * here we want to use the sort algorithm which work on module
> descriptors : so we first put
> > @@ -92,10 +103,8 @@
> > public List sortModuleDescriptors(Collection moduleDescriptors,
> > NonMatchingVersionReporter nonMatchingVersionReporter)
> > throws CircularDependencyException {
> > - VersionMatcher versionMatcher = settings.getVersionMatcher();
> > - CircularDependencyStrategy circularDepStrategy =
> settings.getCircularDependencyStrategy();
> > ModuleDescriptorSorter sorter = new
> ModuleDescriptorSorter(moduleDescriptors,
> > - versionMatcher, nonMatchingVersionReporter,
> circularDepStrategy);
> > + versionMatcher, nonMatchingVersionReporter,
> circularStrategy);
> > return sorter.sortModuleDescriptors();
> > }
> >
> >
> > Modified:
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> > URL:
>
http://svn.apache.org/viewvc/incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java?view=diff&rev=548695&r1=548694&r2=548695
> >
> ==============================================================================
> > ---
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> (original)
> > +++
> incubator/ivy/core/trunk/test/java/org/apache/ivy/core/sort/SortTest.java
> Tue Jun 19 04:32:43 2007
> > @@ -27,7 +27,6 @@
> > import junit.framework.Assert;
> > import junit.framework.TestCase;
> >
> > -import org.apache.ivy.Ivy;
> > import
> org.apache.ivy.core.module.descriptor.DefaultDependencyDescriptor;
> > import org.apache.ivy.core.module.descriptor.DefaultModuleDescriptor;
> > import org.apache.ivy.core.module.descriptor.DependencyDescriptor;
> > @@ -35,6 +34,9 @@
> > import org.apache.ivy.core.module.id.ModuleRevisionId;
> > import org.apache.ivy.plugins.circular.CircularDependencyHelper;
> > import org.apache.ivy.plugins.circular.CircularDependencyStrategy;
> > +import org.apache.ivy.plugins.circular.WarnCircularDependencyStrategy;
> > +import org.apache.ivy.plugins.version.ExactVersionMatcher;
> > +import org.apache.ivy.plugins.version.LatestVersionMatcher;
> >
> > public class SortTest extends TestCase {
> >
> > @@ -46,9 +48,9 @@
> >
> > private DefaultModuleDescriptor md4;
> >
> > - private static Ivy ivy;
> > -
> > -
> > + private SortEngine sortEngine;
> > +
> > + private SilentNonMatchingVersionReporter nonMatchReporter;
> >
> > /*
> > * (non-Javadoc)
> > @@ -63,8 +65,11 @@
> > md3 = createModuleDescriptorToSort("md3", "rev3");
> > md4 = createModuleDescriptorToSort("md4", "rev4");
> >
> > - ivy = new Ivy();
> > - ivy.configureDefault();
> > + sortEngine = new SortEngine();
> > + sortEngine.setCircularDependencyStrategy(
> WarnCircularDependencyStrategy.getInstance());
> > + sortEngine.setVersionMatcher(new ExactVersionMatcher());
> > +
> > + nonMatchReporter = new SilentNonMatchingVersionReporter();
> > }
> >
> > public void testSort() throws Exception {
> > @@ -78,7 +83,7 @@
> > Collection permutations = getAllLists(md1, md3, md2, md4);
> > for (Iterator it = permutations.iterator(); it.hasNext();) {
> > List toSort = (List) it.next();
> > - assertSorted(expectedOrder, ivy.sortModuleDescriptors
> (toSort));
> > + assertSorted(expectedOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > }
> > }
> >
> > @@ -100,7 +105,7 @@
> > Collection permutations = getAllLists(md1, md3, md2, md4);
> > for (Iterator it = permutations.iterator(); it.hasNext();) {
> > List toSort = (List) it.next();
> > - assertSorted(possibleOrder, ivy.sortModuleDescriptors
> (toSort));
> > + assertSorted(possibleOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > }
> > }
> >
> > @@ -117,7 +122,7 @@
> > Collection permutations = getAllLists(md1, md3, md2, md4);
> > for (Iterator it = permutations.iterator(); it.hasNext();) {
> > List toSort = (List) it.next();
> > - assertSorted(possibleOrder, ivy.sortModuleDescriptors
> (toSort));
> > + assertSorted(possibleOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > }
> > }
> >
> > @@ -160,10 +165,10 @@
> > }
> > }
> > CircularDependencyReporterMock circularDepReportMock = new
> CircularDependencyReporterMock();
> > - ivy.getSettings
> ().setCircularDependencyStrategy(circularDepReportMock);
> > + sortEngine.setCircularDependencyStrategy
> (circularDepReportMock);
> >
> > List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3,
> md2, md1});
> > - ivy.sortModuleDescriptors(toSort);
> > + sortEngine.sortModuleDescriptors(toSort, nonMatchReporter);
> >
> > circularDepReportMock.validate();
> > }
> > @@ -178,13 +183,15 @@
> > addDependency(md3, "md2", "latest.integration");
> > addDependency(md4, "md3", "latest.integration");
> >
> > + sortEngine.setVersionMatcher(new LatestVersionMatcher());
> > +
> > DefaultModuleDescriptor[][] expectedOrder = new
> DefaultModuleDescriptor[][] {{md1, md2,
> > md3, md4}};
> >
> > Collection permutations = getAllLists(md1, md3, md2, md4);
> > for (Iterator it = permutations.iterator(); it.hasNext();) {
> > List toSort = (List) it.next();
> > - assertSorted(expectedOrder, ivy.sortModuleDescriptors
> (toSort));
> > + assertSorted(expectedOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > }
> >
> > }
> > @@ -210,7 +217,7 @@
> > Collection permutations = getAllLists(md1, md3, md2, md4);
> > for (Iterator it = permutations.iterator(); it.hasNext();) {
> > List toSort = (List) it.next();
> > - assertSorted(possibleOrder, ivy.sortModuleDescriptors
> (toSort));
> > + assertSorted(possibleOrder,
> sortEngine.sortModuleDescriptors(toSort, nonMatchReporter));
> > }
> >
> > }
> > @@ -244,7 +251,7 @@
> > NonMatchingVersionReporterMock nonMatchingVersionReporterMock =
> > new NonMatchingVersionReporterMock();
> > List toSort = Arrays.asList(new ModuleDescriptor[] {md4, md3,
> md2, md1});
> > - ivy.sortModuleDescriptors(toSort,
> nonMatchingVersionReporterMock);
> > + sortEngine.sortModuleDescriptors(toSort,
> nonMatchingVersionReporterMock);
> > nonMatchingVersionReporterMock.validate();
> > }
> >
> >
> >
> >
>
>
> --
> Gilles SCOKART
>
--
Xavier Hanin - Independent Java Consultant
Manage your dependencies with Ivy!
http://incubator.apache.org/ivy/
