On 16/06/2011 17:32, sebb wrote:
> On 16 June 2011 17:25, Phil Steitz <phil.ste...@gmail.com> wrote:
>> Yesterday I fixed some [dbcp] "problems" caused by the new [pool]
>> requirement that setFactory can only be called once. The quotes are
>> because most of the problems were redundant calls to setFactory.
>> The reason that we left setFactory in [pool] is that [dbcp]'s
>> connection factory constructors call setFactory on the pool passed
>> to them. It is an easy "mistake" to create a pool, then create a
>> connection factory and then do pool.setFactory(factory). I
>> eliminated all of these usages from within [dbcp], but I bet a fair
>> amount of user code will similarly blow up when people upgrade.
>
> AIUI, the upgrade does require code to be changed because of the package
> rename?
It does, but that is just a search and replace and compilation will fail
until it is fixed.
>
> If so, can we not drop the setFactory method and provide it as a
> constructor parameter?
I don't think so. We have been around this buoy already.
To quote Phil's summary:
<quote>
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
</quote>
I disagree with Phil on this. Since a pool only has one factory whereas
a factory may support multiple pools, I think it is wrong for for "new
GKOP(factory,...)" to have "factory.setPool(this)" as a side-effect. I
think "new KPOF(...,pool...)" having "pool.setFactory(this)" as a side
effect is the more natural choice (as per the current code).
>> I hate to keep backsliding here, but maybe we should to this in GOP
>> setFactory:
>>
>> synchronized (factoryLock) {
>> if (this.factory == null) {
>> this.factory = factory;
>> - } else {
>> + } else if (this.factory != factory) {
>> throw new IllegalStateException("Factory
>> already set");
>> }
>> }
I'd be OK with that. If we sort out logging we can log it at INFO as
unnecessary.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
