Details
-
Type:
Improvement
-
Status:
Closed
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 1.0-beta-5, 1.1, 2.0
-
Fix Version/s: 1-1-beta-2
-
Component/s: PicoContainer (Java)
-
Labels:None
-
Environment:all
-
Number of attachments :
Description
There is a cyclic dependency between the ComponentAdapter and PicoContainer interfaces, which is not very nice at all. Worse, ComponentAdapter even depends on MutablePicoContainer.
This is bad design (cyclic == bad), and it creates headaches when writing a custom PicoContainer implementation.
The dependency from ComponentAdapter on MutablePicoContainer and on PicoContainer should be removed. A way to do that is to use some kind of context/resolver/dependency provider interface, have the PicoContainer feed that to the ComponentAdapter, and remove the resonsibility on ComponentAdapters to register themselves with the PicoContainer (which can be seen as subversion of control, in fact).
A design similar to this existed previously (ComponentRegistry).
This has come up several times now. Recent mailing list thread is at:
http://lists.codehaus.org/pipermail/picocontainer-dev/2003-December/001913.html
Activity
yeah, I pretty much agree with your damage assessment 
. Holding up a 1.0 release over a decomposition issue is probably not a good idea. The tricky thing is that I guesstimate that changing this later will either totally break all BWC or double the API size and do at least the same for the implementation size.
Leo Simons
added a comment - yeah, I pretty much agree with your damage assessment . Holding up a 1.0 release over a decomposition issue is probably not a good idea. The tricky thing is that I guesstimate that changing this later will either totally break all BWC or double the API size and do at least the same for the implementation size. my vote: do it before v1.0 is out 
The damage done will only grow after a v1 is out and it isnt even a that big change from the code perspective. If you move the registerComponentImpl/Inst* methods out of MutablePicoContainer into a new one and implement it you can use it in DefaultPicoContainer. Even make a base container that doesnt use it and an extended version that does it (SimplePicoContainer or something). The ComponentAdapter damage NOW should be very limited since everyone that is using ComponentAdapterFactorys should have seen them anyways.
just 2 cent
Thomas Heller
added a comment - my vote: do it before v1.0 is out
The damage done will only grow after a v1 is out and it isnt even a that big change from the code perspective. If you move the registerComponentImpl/Inst* methods out of MutablePicoContainer into a new one and implement it you can use it in DefaultPicoContainer. Even make a base container that doesnt use it and an extended version that does it (SimplePicoContainer or something). The ComponentAdapter damage NOW should be very limited since everyone that is using ComponentAdapterFactorys should have seen them anyways.
just 2 cent I agree, I think if we can we should do this before 1.0.
That really depends on how big the change is though. If it's too big we shouldn't do it.
I'll have a look at it and to see whether you can do it without rewriting the whole damn thing. Report back here later.
We have our only chance of getting it somewhat correct now, when we release 1.0 we will have to live with our misstakes for a long time.
Jon Tirsen
added a comment - I agree, I think if we can we should do this before 1.0.
That really depends on how big the change is though. If it's too big we shouldn't do it.
I'll have a look at it and to see whether you can do it without rewriting the whole damn thing. Report back here later.
We have our only chance of getting it somewhat correct now, when we release 1.0 we will have to live with our misstakes for a long time. Aslak Hellesøy
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Affects Version/s | 2.0 [ 10411 ] | |
| Affects Version/s | 1.0.1 [ 10307 ] | |
| Assignee | Jon Tirsen [ tirsen ] | |
| Fix Version/s | 2.0 [ 10411 ] |
Aslak Hellesøy
made changes -
| Fix Version/s | 2.0 [ 10411 ] | |
| Fix Version/s | 1.0 [ 10145 ] |
Jon Tirsen
made changes -
| Fix Version/s | 1.0-beta-4 [ 10412 ] | |
| Fix Version/s | 1.0 [ 10145 ] |
Jon Tirsen
made changes -
| Priority | Major [ 3 ] | Blocker [ 1 ] |
Jon Tirsen
made changes -
| Priority | Blocker [ 1 ] | Minor [ 4 ] |
Aslak Hellesøy
made changes -
| Fix Version/s | 1.0-beta-4 [ 10412 ] | |
| Fix Version/s | 1.0 [ 10145 ] |
Aslak Hellesøy
made changes -
| Fix Version/s | 1.0-beta-5 [ 10145 ] | |
| Fix Version/s | 1.0-RC-1 [ 10461 ] |
you guys sure you want to do this before 1.0? I could write a patch, probably, but it will probably be a really big one.
Leo Simons
added a comment - you guys sure you want to do this before 1.0? I could write a patch, probably, but it will probably be a really big one. We'll definitely not do this before 1.0. If ever.
Aslak Hellesøy
added a comment - We'll definitely not do this before 1.0. If ever. that's what I thought :-D
Leo Simons
added a comment - that's what I thought :-D Aslak Hellesøy
made changes -
| Fix Version/s | 1.0.1 [ 10307 ] | |
| Fix Version/s | 1.0-RC-1 [ 10461 ] |
The dependency of CA on PC has been removed with MX_PROPOSAL.
Jörg Schaible
added a comment - The dependency of CA on PC has been removed with MX_PROPOSAL. Jörg Schaible
made changes -
| Fix Version/s | 1-1-beta-2 [ 11243 ] | |
| Resolution | Fixed [ 1 ] | |
| Assignee | Jon Tirsen [ tirsen ] | Joerg Schaible [ joehni ] |
| Fix Version/s | 1.1 [ 10307 ] | |
| Status | Open [ 1 ] | Closed [ 6 ] |
The cyclic dependency between ComponentAdapter and (Mutable)PicoContainer is on the method parameter level, and not on the instance level. This is not the worst kind of mutual dependency in the world, and we can live with it for now.
However, I agree that this should be fixed. We can certainly make PicoContainer more IoC by introducing some kind of ComponentLookup interface.
I am positive about this. But I think it should be postponed until after the release of PicoContainer 1.0, preferrably on a 2.0 branch as this will break BWC.