On 3/10/06, Phil Steitz <[EMAIL PROTECTED]> wrote:
> Any comments on this?
> http://issues.apache.org/bugzilla/show_bug.cgi?id=38073
>
> As I see it, there are two decent alternatives here.  The one I first
> suggested in the report is too ugly.
>
> (i) Make the (incorrectly) implemenented getReference method in
> InstanceKeyDataSource throw, as described in the last comment on the
> report, and override with correct impls in the concrete subclasses.
>
> (ii) Make the (minor) backward-incompatible change to make the
> offending method in InstanceKeyDataSource abstract, as suggested in
> the original report, again providing correct impls in the subclasses.
>
> I think (ii) is cleaner (what should have been done in the first
> place) but is technically a backward incompatible change.  Users who
> have subclassed will be forced to implement.
>
> Interested parties, please weigh in with opinions or better ideas
> about how to resolve this.

(Standard disclaimer about not being familiar with Dbcp yet.)

For a bug fix release it seems to me that you should override
getReference() in PerUserPoolDataSource and SharedPoolDataSource. And
document in InstanceKeyDataSource.getReference() that it will be going
away in the next major release.

Also, it seems to me that changing
InstanceKeyDataSource.getReference() for the ref var to:
        Reference ref = new Reference(getClass().getName(),
            getClass().getName() + "Factory", null);

would work better. It's fragile but I don't think it would break anything new.

> Also scream now if adding test
> dependencies on the (ibiblio published) tomcat naming jars is not
> acceptable.

For building and testing only right? Not for deployment of Dbcp?

--
Sandy McArthur

"He who dares not offend cannot be honest."
- Thomas Paine

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to