On 5/22/11 2:01 PM, Mark Thomas wrote: > All, > > Apologies for the long e-mail. I'll keep it as short as I can. > > I am working on dbcp2 alongside pool2, keeping the two in sync as pool2 > is refactored. I am currently looking at the removal of the setFactory() > method from G(K)OP and it isn't pretty. > > Removing G(K)OP.setFactory() from pool2 is straight forward. Just a few > getFactory() methods to remove from sub-classes and a couple of simple > deletes in the test classes. > > The impact on dbcp2 is more problematic. It is ugly for GOP and > significantly worse for GKOP. There are multiple cases in DBCP of > (Keyed)PoolableObjectFactory retaining references to the > (Keyed)ObjectPool they are a factory for. The current approach is to > pass the (Keyed)ObjectPool in to the constructor of the > (Keyed)PoolableObjectFactory and then call setFactory() on the pool.
Personally, I have always found that code confusing. IIRC in some cases findbugs gets confused by it too ;) > Removing the setFactory() method makes this impossible. > > My first thought was to create the (Keyed)PoolableObjectFactory without > the (Keyed)ObjectPool and then call a setPool() method. This is doable > but it is going to require a lot of changes and I am not sure it is > worth the effort. I also looked at this before and came to the same conclusion. I think it is more natural to have pool = new GKOP(factory,...) have factory.setPool(this) as a side effect than factory = new KPOF(...,pool...) have pool.setFactory(this) as a side effect, but it's messy either way, so I agree with you it is not worth the effort to change. > The reason the setFactory() is being removed is to make the Factory > immutable. I'm beginning to think that a better approach would be to > keep the setFactory() method but modify it so that it can only be called > once. My current thinking is that calls to setFactory() throw > IllegalStateException if _factory != null. That will need some > double-checked locking to get right but that is safely doable in 1.5 > onwards. > > This alternative approach would make my life considerably easier. +1 - and likely applies to some (all?) other BDS properties. Phil > Thoughts, comments? > > Cheers, > > Mark > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
