Issue Details (XML | Word | Printable)

Key: PICO-202
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Minor Minor
Assignee: Joerg Schaible
Reporter: Nick Sieger
Votes: 0
Watchers: 0
Operations

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

GenericCollectionComponentAdapter with composite pattern

Created: 04/Aug/04 11:06 AM   Updated: 04/Aug/04 10:05 PM
Component/s: PicoContainer (Java)
Affects Version/s: 1.0
Fix Version/s: 1.1

Time Tracking:
Not Specified

File Attachments: 1. Text File GenericCollectionComponentAdapter.patch (5 kB)
2. Text File GenericCollectionComponentAdapter.patch (5 kB)



 Description  « Hide
I would like to use GenericCollectionComponentAdapter to inject a composite comp with an array of children components in an instance of the composite pattern. The attached patch has a couple of points:

1. Before I could successfully resolve dependencies to the array I had to make the change to the GCCA to pass the collection type to the AbstractCA. I added the collectionType.isArray() check which seemed prudent. Original test case was modified to pass String[].class for the collection type and it still passes.

2. For the second test case, I had to tweak the container config for a composite since the composite itself is an instance of the component I'm aggregating in the array. The test is more of an integration test since I'm using a container (I realize tests using the container are discouraged), but I wanted to test the container lookup logic with the GCCA present. In order to avoid the cyclic dependency of the array->CompositeComponent->array, I had to wrap a caching CA around the GCCA and pre-instantiate the array before registering the composite. Not ideal, but it works.

Does this approach seem reasonable or is there a better way?



 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Nick Sieger added a comment - 04/Aug/04 11:09 AM
Cleaned up imports in the second version

Joerg Schaible added a comment - 04/Aug/04 11:11 AM
I did yesterday a quite similar modification to GenericCollectionCA in the MX_PROPOSAL branch. I'll have a look.

Nick Sieger added a comment - 04/Aug/04 11:12 AM
Another approach would be to register the child comps and the array/GCCA in a parent container and put the composite in a child container, but using two containers might seem like overkill.

Joerg Schaible added a comment - 04/Aug/04 11:18 AM
Not really. The arrays have to be generated on the fly, since they may change ... unless the container is read-only.

Nick Sieger added a comment - 04/Aug/04 02:02 PM
In my haste I didn't even realize that GCCA is a package-private class only used by the CICA! So the testInstantiateCompositeDependingOnComponentArrayRequiresPreInstantiationAndCaching case wouldn't even normally work. I still end up having to create the array manually and registering it as a component instance. I basically end up with this, which now does not even leverage GCCA:

public void testInstantiateCompositeDependingOnComponentArrayRequiresPreInstantiationAndCaching() {
MutablePicoContainer pc = new DefaultPicoContainer();
pc.registerComponentImplementation(LeafComponent.class);
pc.registerComponentInstance("apple");
Collection comps = pc.getComponentInstancesOfType(Component.class);
assertEquals(1, comps.size());
pc.registerComponentInstance("x", comps.toArray());

Object[] arr = (Object[]) pc.getComponentInstance("x");

assertNotNull(arr);
assertEquals(1, arr.length);

pc.registerComponentImplementation(CompositeComponent.class,
CompositeComponent.class,
new Parameter[] {new ComponentParameter("x")});

Component cc = (Component) pc.getComponentInstance(CompositeComponent.class);

assertNotNull(cc);
assertEquals("apple", cc.toString());
}


Nick Sieger added a comment - 04/Aug/04 02:17 PM
So the patch isn't really valid.

Even so, it would still be nice for the following to just work with minimal extra config. I guess this is related to Thomas' comments on the list about the greedy nature of the GCCA – see http://article.gmane.org/gmane.comp.java.picocontainer.devel/3491. If I have any ideas for improvement I'll speak up on the list or submit another patch.

public static interface Component {}

public static class CompositeComponent implements Component {
Object[] children;

public CompositeComponent(Object[] children) { this.children = children; }
}

public static class LeafComponent implements Component {
public LeafComponent() {
}
}

pc.registerComponentInstance(LeafComponent.class);
pc.registerComponentInstance(CompositeComponent.class);


Joerg Schaible added a comment - 04/Aug/04 10:05 PM
Have a look at the GenericCollectionAdapterTestCase. This should demonstrate, what you want.