PicoContainer
  1. PicoContainer
  2. PICO-61

Lazy assignment in TransientComponentAdapter is too lazy

    Details

    • Type: Improvement Improvement
    • Status: Closed Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.0-beta-4
    • Component/s: PicoContainer (Java)
    • Labels:
      None
    • Environment:
      All
    • Number of attachments :
      0

      Description

      I was looking around PicoContainer to see how out-of-date Rico was, and noticed there were lots of parameter names masking field names. So I went through and did some weeding, and noticed that the name masking was causing the parameters field in TransientComponentAdapter to never be assigned, so it would always be calculated.

      Also removed some redundant semicolons and other trivia.

      Impact: all tests that were running are still passing. No new tests added. Cleaned up some unused variables.

      Below is a patch (should be run from inside src):

      --------8<-----------8<-----------8<-----------------

      Index: java/org/picocontainer/defaults/AbstractComponentAdapter.java
      ===================================================================
      RCS file: /cvsroot/picocontainer/pico/src/java/org/picocontainer/defaults/AbstractComponentAdapter.java,v
      retrieving revision 1.6
      diff -u -r1.6 AbstractComponentAdapter.java
      — java/org/picocontainer/defaults/AbstractComponentAdapter.java 2 Nov 2003 11:06:22 -0000 1.6
      +++ java/org/picocontainer/defaults/AbstractComponentAdapter.java 12 Nov 2003 12:34:09 -0000
      @@ -18,10 +18,10 @@
      if(componentImplementation == null)

      { throw new NullPointerException("componentImplementation"); }
      • checkTypeCompatibility(componentKey, componentImplementation);
      • checkConcrete(componentImplementation);
        this.componentKey = componentKey;
        this.componentImplementation = componentImplementation;
        + checkTypeCompatibility();
        + checkConcrete();
        }

      public Object getComponentKey()

      { @@ -35,7 +35,7 @@ return componentImplementation; }
      • private void checkTypeCompatibility(Object componentKey, Class componentImplementation) throws AssignabilityRegistrationException {
        + private void checkTypeCompatibility() throws AssignabilityRegistrationException {
        if (componentKey instanceof Class)
        Unknown macro: { Class componentType = (Class) componentKey; if (!componentType.isAssignableFrom(componentImplementation)) { @@ -44,7 +44,7 @@ } }
      • private void checkConcrete(Class componentImplementation) throws NotConcreteRegistrationException {
        + private void checkConcrete() throws NotConcreteRegistrationException {
        // Assert that the component class is concrete.
        boolean isAbstract = (componentImplementation.getModifiers() & Modifier.ABSTRACT) == Modifier.ABSTRACT;
        if (componentImplementation.isInterface() || isAbstract) { Index: java/org/picocontainer/defaults/CyclicDependencyException.java =================================================================== RCS file: /cvsroot/picocontainer/pico/src/java/org/picocontainer/defaults/CyclicDependencyException.java,v retrieving revision 1.2 diff -u -r1.2 CyclicDependencyException.java --- java/org/picocontainer/defaults/CyclicDependencyException.java 11 Oct 2003 15:37:56 -0000 1.2 +++ java/org/picocontainer/defaults/CyclicDependencyException.java 12 Nov 2003 12:34:09 -0000 @@ -21,7 +21,7 @@ return "Cyclic dependency: " + constructor.getName() + "(" + getCtorParams(constructor) + ")"; }
      • private String getCtorParams(Constructor constructor) {
        + private static String getCtorParams(Constructor constructor) {
        String retval = "";
        Class[] parameterTypes = constructor.getParameterTypes();
        for (int i = 0; i < parameterTypes.length; i++) {
        Index: java/org/picocontainer/defaults/DefaultComponentMulticasterFactory.java
        ===================================================================
        RCS file: /cvsroot/picocontainer/pico/src/java/org/picocontainer/defaults/DefaultComponentMulticasterFactory.java,v
        retrieving revision 1.6
        diff -u -r1.6 DefaultComponentMulticasterFactory.java
          • java/org/picocontainer/defaults/DefaultComponentMulticasterFactory.java 20 Oct 2003 16:36:32 -0000 1.6
            +++ java/org/picocontainer/defaults/DefaultComponentMulticasterFactory.java 12 Nov 2003 12:34:09 -0000
            @@ -15,10 +15,6 @@
            import java.util.List;
            import java.util.ArrayList;
            import java.util.Collections;
            -import java.util.Set;
            -import java.util.HashSet;
            -import java.util.Arrays;
            -import java.util.Iterator;
            import java.lang.reflect.Proxy;
            import java.lang.reflect.InvocationHandler;
            import java.lang.reflect.Method;
            Index: java/org/picocontainer/defaults/TransientComponentAdapter.java
            ===================================================================
            RCS file: /cvsroot/picocontainer/pico/src/java/org/picocontainer/defaults/TransientComponentAdapter.java,v
            retrieving revision 1.2
            diff -u -r1.2 TransientComponentAdapter.java
          • java/org/picocontainer/defaults/TransientComponentAdapter.java 3 Nov 2003 17:58:29 -0000 1.2
            +++ java/org/picocontainer/defaults/TransientComponentAdapter.java 12 Nov 2003 12:34:10 -0000
            @@ -64,13 +64,12 @@
            private Constructor getConstructor(MutablePicoContainer picoContainer) throws PicoIntrospectionException, NoSatisfiableConstructorsException, AmbiguousComponentResolutionException, AssignabilityRegistrationException, NotConcreteRegistrationException {
            if (greediestConstructor == null) {
            List allConstructors = Arrays.asList(getComponentImplementation().getConstructors());
      • List satisfiableConstructors = getSatisfiableConstructors(allConstructors, picoContainer);

      // now we'll just take the biggest one
      greediestConstructor = null;
      Set conflicts = new HashSet();

      • for (int i = 0; i < satisfiableConstructors.size(); i++) {
      • Constructor currentConstructor = (Constructor) satisfiableConstructors.get;
        + for (Iterator i = getSatisfiableConstructors(allConstructors, picoContainer).iterator(); i.hasNext() {
        + Constructor currentConstructor = (Constructor) i.next();
        if (greediestConstructor == null) { greediestConstructor = currentConstructor; }

        else if (greediestConstructor.getParameterTypes().length < currentConstructor.getParameterTypes().length)

        { @@ -211,14 +210,6 @@ return actual.isAssignableFrom(requested); }
      • private Parameter[] getParameters(MutablePicoContainer componentRegistry) throws PicoIntrospectionException, AssignabilityRegistrationException, NotConcreteRegistrationException {
      • if (parameters == null) { - return createDefaultParameters(getDependencies(componentRegistry)); - }

        else

        { - return parameters; - }
      • }
        -
        public boolean equals(Object object) {
        if (!(object instanceof ComponentAdapter)) { return false; @@ -229,10 +220,17 @@ getComponentImplementation().equals(other.getComponentImplementation()); }
      • private Parameter[] createDefaultParameters(Class[] parameters) {
      • ComponentParameter[] componentParameters = new ComponentParameter[parameters.length];
      • for (int i = 0; i < parameters.length; i++) {
      • componentParameters[i] = new ComponentParameter(parameters[i]);
        + private Parameter[] getParameters(MutablePicoContainer componentRegistry) throws PicoIntrospectionException, AssignabilityRegistrationException, NotConcreteRegistrationException
        Unknown macro: {+ if (parameters == null) { + parameters = createDefaultParameters(getDependencies(componentRegistry)); + }+ return parameters;+ }

        +
        + private Parameter[] createDefaultParameters(Class[] parameterClasses)

        Unknown macro: {+ ComponentParameter[] componentParameters = new ComponentParameter[parameterClasses.length];+ for (int i = 0; i < parameterClasses.length; i++) { + componentParameters[i] = new ComponentParameter(parameterClasses[i]); } return componentParameters; }

        Index: java/org/picocontainer/extras/ImplementationHidingComponentAdapterFactory.java
        ===================================================================
        RCS file: /cvsroot/picocontainer/pico/src/java/org/picocontainer/extras/ImplementationHidingComponentAdapterFactory.java,v
        retrieving revision 1.11
        diff -u -r1.11 ImplementationHidingComponentAdapterFactory.java

          • java/org/picocontainer/extras/ImplementationHidingComponentAdapterFactory.java 20 Oct 2003 16:36:32 -0000 1.11
            +++ java/org/picocontainer/extras/ImplementationHidingComponentAdapterFactory.java 12 Nov 2003 12:34:11 -0000
            @@ -76,5 +76,5 @@
            return method.invoke(componentInstance, args);
            }
            }
      • };
        + }
        }
        Index: test/org/picocontainer/defaults/DefaultPicoContainerTestCase.java
        ===================================================================
        RCS file: /cvsroot/picocontainer/pico/src/test/org/picocontainer/defaults/DefaultPicoContainerTestCase.java,v
        retrieving revision 1.23
        diff -u -r1.23 DefaultPicoContainerTestCase.java
          • test/org/picocontainer/defaults/DefaultPicoContainerTestCase.java 30 Oct 2003 21:53:13 -0000 1.23
            +++ test/org/picocontainer/defaults/DefaultPicoContainerTestCase.java 12 Nov 2003 12:34:11 -0000
            @@ -12,11 +12,7 @@

      import org.picocontainer.*;
      import org.picocontainer.testmodel.Touchable;
      -import org.picocontainer.testmodel.DependsOnTouchable;
      import org.picocontainer.tck.AbstractPicoContainerTestCase;
      -
      -import java.util.Map;
      -import java.io.Serializable;

      public class DefaultPicoContainerTestCase extends AbstractPicoContainerTestCase {

      Index: test/org/picocontainer/defaults/OldDefaultPicoContainerTestCase.java
      ===================================================================
      RCS file: /cvsroot/picocontainer/pico/src/test/org/picocontainer/defaults/OldDefaultPicoContainerTestCase.java,v
      retrieving revision 1.19
      diff -u -r1.19 OldDefaultPicoContainerTestCase.java
      — test/org/picocontainer/defaults/OldDefaultPicoContainerTestCase.java 30 Oct 2003 21:53:13 -0000 1.19
      +++ test/org/picocontainer/defaults/OldDefaultPicoContainerTestCase.java 12 Nov 2003 12:34:13 -0000
      @@ -18,7 +18,6 @@
      import org.picocontainer.testmodel.Touchable;
      import org.picocontainer.testmodel.Webster;

      -import java.io.Serializable;
      import java.lang.reflect.UndeclaredThrowableException;
      import java.util.*;

      @@ -164,7 +163,6 @@
      (CoincidentallyPeelableComponent) pico.getComponentInstance(CoincidentallyPeelableComponent.class);
      PeelableAndWashableComponent washAndPeel =
      (PeelableAndWashableComponent) pico.getComponentInstance(PeelableAndWashableComponent.class);

      • ;

      assertFalse(washAndPeel.wasPeeled);
      assertTrue(washAndPeel.wasWashed);
      Index: test/org/picocontainer/extras/DelegatingPicoContainerTestCase.java
      ===================================================================
      RCS file: /cvsroot/picocontainer/pico/src/test/org/picocontainer/extras/DelegatingPicoContainerTestCase.java,v
      retrieving revision 1.4
      diff -u -r1.4 DelegatingPicoContainerTestCase.java
      — test/org/picocontainer/extras/DelegatingPicoContainerTestCase.java 17 Sep 2003 23:57:02 -0000 1.4
      +++ test/org/picocontainer/extras/DelegatingPicoContainerTestCase.java 12 Nov 2003 12:34:14 -0000
      @@ -41,7 +41,7 @@
      child.registerComponentImplementation(SimpleTouchable.class);
      parent.registerComponentImplementation(DependsOnTouchable.class);
      try

      { - DependsOnTouchable dependsOnTouchable = (DependsOnTouchable) parent.getComponentInstance(DependsOnTouchable.class); + parent.getComponentInstance(DependsOnTouchable.class); fail(); }

      catch (NoSatisfiableConstructorsException e) {
      }
      Index: test/org/picocontainer/extras/ImplementationHidingComponentAdapterFactoryTestCase.java
      ===================================================================
      RCS file: /cvsroot/picocontainer/pico/src/test/org/picocontainer/extras/ImplementationHidingComponentAdapterFactoryTestCase.java,v
      retrieving revision 1.8
      diff -u -r1.8 ImplementationHidingComponentAdapterFactoryTestCase.java
      — test/org/picocontainer/extras/ImplementationHidingComponentAdapterFactoryTestCase.java 30 Oct 2003 21:53:13 -0000 1.8
      +++ test/org/picocontainer/extras/ImplementationHidingComponentAdapterFactoryTestCase.java 12 Nov 2003 12:34:14 -0000
      @@ -18,8 +18,6 @@

      public class ImplementationHidingComponentAdapterFactoryTestCase extends AbstractComponentAdapterFactoryTestCase {

      • private static boolean addCalled = false;
        -
        public static class ReferencesSwappables {
        public final Swappable swappable;

      Index: test/org/picocontainer/tck/AbstractMultipleConstructorTestCase.java
      ===================================================================
      RCS file: /cvsroot/picocontainer/pico/src/test/org/picocontainer/tck/AbstractMultipleConstructorTestCase.java,v
      retrieving revision 1.4
      diff -u -r1.4 AbstractMultipleConstructorTestCase.java
      — test/org/picocontainer/tck/AbstractMultipleConstructorTestCase.java 1 Oct 2003 23:06:15 -0000 1.4
      +++ test/org/picocontainer/tck/AbstractMultipleConstructorTestCase.java 12 Nov 2003 12:34:14 -0000
      @@ -1,7 +1,6 @@
      package org.picocontainer.tck;

      import junit.framework.TestCase;
      -import org.picocontainer.PicoInitializationException;
      import org.picocontainer.PicoRegistrationException;
      import org.picocontainer.MutablePicoContainer;
      import org.picocontainer.PicoException;
      @@ -79,7 +78,7 @@
      pico.registerComponentImplementation(Two.class);

      try

      { - Multi multi = (Multi) pico.getComponentInstance(Multi.class); + pico.getComponentInstance(Multi.class); fail(); }

      catch (TooManySatisfiableConstructorsException e) {
      assertTrue(e.getMessage().indexOf("Three") == -1);
      Index: test/org/picocontainer/tck/AbstractPicoContainerTestCase.java
      ===================================================================
      RCS file: /cvsroot/picocontainer/pico/src/test/org/picocontainer/tck/AbstractPicoContainerTestCase.java,v
      retrieving revision 1.11
      diff -u -r1.11 AbstractPicoContainerTestCase.java
      — test/org/picocontainer/tck/AbstractPicoContainerTestCase.java 4 Nov 2003 12:06:21 -0000 1.11
      +++ test/org/picocontainer/tck/AbstractPicoContainerTestCase.java 12 Nov 2003 12:34:15 -0000
      @@ -147,7 +147,7 @@
      pico.registerComponentImplementation("a", ArrayList.class);
      pico.registerComponentImplementation("l", LinkedList.class);

      • ListAdder adder = (ListAdder) pico.getComponentInstance(ListAdder.class);
        + pico.getComponentInstance(ListAdder.class);

      List a = (List) pico.getComponentInstance("a");
      assertTrue(a.contains("something"));
      @@ -158,9 +158,9 @@

      public static class A {
      public A(B b, C c){}

      • };
      • public static class B {};
      • public static class C {};
        + }
        + public static class B {}
        + public static class C {}

      public void testUnsatisfiedComponentsExceptionGivesVerboseEnoughErrorMessage() {
      MutablePicoContainer pico = createPicoContainer();
      @@ -179,10 +179,10 @@

      public static class D {
      public D(E e, B b){}

      • };
        + }
        public static class E {
        public E(D d){}
      • };
        + }

      public void testCyclicDependencyThrowsCyclicDependencyException() {
      MutablePicoContainer pico = createPicoContainer();

        Activity

        Aslak Hellesøy made changes -
        Field Original Value New Value
        Assignee Aslak Hellesoy [ rinkrank ]
        Hide
        Aslak Hellesøy added a comment -

        Fixed. Nice one.

        Show
        Aslak Hellesøy added a comment - Fixed. Nice one.
        Aslak Hellesøy made changes -
        Resolution Fixed [ 1 ]
        Fix Version/s 1.0 [ 10145 ]
        Status Open [ 1 ] Closed [ 6 ]
        Aslak Hellesøy made changes -
        Resolution Fixed [ 1 ]
        Status Closed [ 6 ] Reopened [ 4 ]
        Aslak Hellesøy made changes -
        Status Reopened [ 4 ] Closed [ 6 ]
        Fix Version/s 1.0-beta-4 [ 10412 ]
        Resolution Fixed [ 1 ]
        Fix Version/s 1.0 [ 10145 ]

          People

          • Assignee:
            Aslak Hellesøy
            Reporter:
            Dan North
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 4 hours
              4h
              Remaining:
              Remaining Estimate - 4 hours
              4h
              Logged:
              Time Spent - Not Specified
              Not Specified