PicoContainer
  1. PicoContainer
  2. PICO-352

Inconsistent findAdapterOfType implementation

          Details

          • Type: Bug Bug
          • Status: Closed Closed
          • Priority: Major Major
          • Resolution: Fixed
          • Affects Version/s: 2.8
          • Fix Version/s: 2.9
          • Component/s: PicoContainer (Java)
          • Labels:
            None
          • Environment:
            Java 1.6 on Windows XP (but that's really irrelevant here)
          • Testcase included:
            yes
          • Patch Submitted:
            Yes
          • Number of attachments :
            0

            Description

            AbstractAdapter.findComponentAdapterOfType implements wrongly as

            AbstractAdapter.java - broken
                   if (this.getClass().isAssignableFrom(componentAdapterType)) {
    		return (U) this;
    	} else if (getDelegate() !=  null) {
    		return getDelegate().findAdapterOfType(componentAdapterType);
    	}
        return null;

            but AbstractBehavior implements this as

            AbstractBehavior.java - ok
                    if (componentAdapterType.isAssignableFrom(this.getClass())) {
            return (U) this;
        // next else block is superfluous
        } else if (componentAdapterType.isAssignableFrom(delegate.getClass())) {
            return (U) delegate;
        } else {
            return delegate.findAdapterOfType(componentAdapterType);
        }

            The first clause is reversed with respect to between the two constructs. In addition, the Behavior implementation's marked else clause is superfluous.

            The junit test code below makes the difference stand out

            Test code
                   public static class Foo {
		public Foo() {
			// empty
		}
	}
	
	// This test fails
	@Test
	public void testShouldFindSupertypeOfAdapterOnAbstractAdapterDerivative() {
		ConstructorInjector<Foo> injector = new ConstructorInjector<Foo>("key", Foo.class);
		assertSame(injector, injector.findAdapterOfType(SingleMemberInjector.class));
	}

	// This test works
	@Test
	public void testShouldFindSupertypeOfAdapterOnAbstractBehaviorDerivative() {
		ConstructorInjector<Foo> injector = new ConstructorInjector<Foo>("key", Foo.class);
		Cached<Foo> adapter = new Cached<Foo>(injector);
		assertSame(adapter, adapter.findAdapterOfType(Stored.class));
		assertSame(injector, adapter.findAdapterOfType(SingleMemberInjector.class));
	}

            I suggest to align the AbstractAdapter implementation with Behavior's in this way

            AbstractAdapter.java - fixed
                    if (adapterType.isAssignableFrom(this.getClass())) {
    		return (U) this;
    	} else if (getDelegate() !=  null) {
    		return getDelegate().findAdapterOfType(componentAdapterType);
    	}
        return null;

            and to simplify Behavior's implementation to

            AbstractBehavior.java - simplified
                    if (componentAdapterType.isAssignableFrom(this.getClass())) {
            return (U) this;
        } else {
            return delegate.findAdapterOfType(componentAdapterType);
        }

              People

              • Assignee:
                Paul Hammant
                Reporter:
                Mark J. Sinke
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: