Mailing-List: contact derby-dev-help@db.apache.org; run by ezmlm
Precedence: bulk
Reply-To: <derby-dev@db.apache.org>
Message-ID: <1366070773.1143157400761.JavaMail.jira@ajax>
Date: Thu, 23 Mar 2006 23:43:20 +0000 (GMT)
From: "Daniel John Debrunner (JIRA)" <derby-dev@db.apache.org>
To: derby-dev@db.apache.org
Subject: [jira] Commented: (DERBY-1137) Implement the new method introduced
 in CommonDataSource for Embedded Driver
In-Reply-To: <68196494.1143015062256.JavaMail.jira@ajax>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit

    [ http://issues.apache.org/jira/browse/DERBY-1137?page=comments#action_12371655 ] 

Daniel John Debrunner commented on DERBY-1137:
----------------------------------------------

I'm looking at this patch. Thanks for the good explanation.

I am concerned by the number of methods that have become public, which allows applications to get underlying information,
but I need to study the patch a little more to see why they became public.

Also it's unclear to me why some of the new classes, such as EmbedPooledConnection40 are public, is that required?

InternalXAResource says it's an implementation of XAResource, but it isn't. The class does not implement that interface.
I would actually think it would be much cleaner if InternalXAResource did implement XAResource, and then the XAConnection
implementation's used an instance of it, rather than implementing XAResource themself. I would recommend this approach
and calling the class EmbedXAResource to match the existing naming scheme. This may have a knock on approach that
InternalXAConnection is not required, but I haven't looked into that fully yet.

EmbeddedXADataSource40 and EmbeddedConnectionPoolDataSource40 follow the existing model that they also implement
DataSource, but we have had concerns in the past that they should not. I wonder if JDBC 4.0 would be a good time to break
that link, and have these two classes not implement javax.sql.DataSource.

To my thinking this is a possibly a case where the incremental development could have been used. For example re-factoring
EmbedXAResource is one change, refactoring the code to use InternalXAConnection another and then finally adding the 4.0 classes.
With the one big patch it's hard to see why a specific change was made, which re-factoring was driving it. I guess this is just a plea
to think about such issues on future changes.

Minor comment on modifying javadoc comments: In EmbeddedDataSource you have this:

+    /**
+     * Moved this code from EmbeddedXADatasource to 
+     * share the method between different versions of XADataSource           
+     * This method attenpts to create resource adapter to database

The primary description for a method (or any javadoc comment) is the first sentence in the Javadoc, which means this method
will now be described as "Moved this code from EmbeddedXADatasource to 
+     * share the method between different versions of XADataSource  ...."
which is not its function.  The first sentence should be the purpose of the method, I'm not sure it's even useful having comments that it moved.


> Implement the new method introduced in CommonDataSource for Embedded Driver
> ---------------------------------------------------------------------------
>
>          Key: DERBY-1137
>          URL: http://issues.apache.org/jira/browse/DERBY-1137
>      Project: Derby
>         Type: Sub-task
>   Components: JDBC
>  Environment: jdk1.6 jdbc4.0
>     Reporter: Anurag Shekhar
>     Assignee: Anurag Shekhar
>      Fix For: 10.2.0.0
>  Attachments: derby-1137.diff, derby-1137_2.diff, stat.out
>


-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira