On 6/20/07, Gilles Scokart <[EMAIL PROTECTED]> wrote:

I now submit the other aproach : using a dedicated settings interface
for each settings.

This has a more limited impact on the current implementation (the Ivy
facade is not impacted).  Si I think this aproach might be better.

Note that I put the SortEngineSettings in the sort package so that the
engine is really isolated from the full settings.  However, it will
make the settings package dependent on all engines package.  I don't
think it is a problem,  but WDYT?


It's ok for me, I think it's cleaner like that.

Xavier

Gilles

2007/6/19, Xavier Hanin <[EMAIL PROTECTED]>:
> 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/
>


--
Gilles SCOKART




--
Xavier Hanin - Independent Java Consultant
Manage your dependencies with Ivy!
http://incubator.apache.org/ivy/

Reply via email to