Issue Details (XML | Word | Printable)

Key: PICO-352
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Paul Hammant
Reporter: Mark J. Sinke
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
PicoContainer

Inconsistent findAdapterOfType implementation

Created: 08/May/09 09:57 AM   Updated: 12/May/09 07:11 AM   Resolved: 12/May/09 07:11 AM
Return to search
Component/s: PicoContainer (Java)
Affects Version/s: 2.8
Fix Version/s: 2.9

Time Tracking:
Not Specified

Environment: Java 1.6 on Windows XP (but that's really irrelevant here)

Testcase included: yes
Patch Submitted: Yes


 Description  « Hide

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);
        }


There are no comments yet on this issue.