commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <phil.ste...@gmail.com>
Subject Re: [DBCP][POOL] Re-factoring headaches with dbcp2 & pool2
Date Mon, 23 May 2011 00:15:56 GMT
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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message