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