I have not been following this patch, but I think Knut-Anders description of client packages is right. During Derby client development, it was the practice we tried to follow... and good to maintain.
I wish I had more time to review JDBC 4.0 patches. Satheesh Knut Anders Hatlen wrote: >Hi, > >I have one additional comment to the patch. > >ClientJDBCObjectFactory, ClientJDBCObjectFactoryImpl and >ClientJDBCObjectFactoryImpl40 are in the org.apache.derby.jdbc >package. I think only classes that are part of our external interface >should be part of that package. The classes that are currently there >are ClientDriver, ClientDataSource and >ClientConnectionPoolDataSource. I don't feel the factory classes >belong there. I also think it's a problem that the constructor for the >NetResultSet class must be made public because the factory >implementation is in the "wrong" package. > >The package hierarchy in the client driver is something like this (I >apologize to the client driver experts if my explanation is a >inaccurate): > > * org.apache.derby.jdbc which contains the external interface, > * org.apache.derby.client.am with the internal interface, and > * org.apache.derby.client.net with the actual (protocol-specific) > implementation > >My feeling is that the ClientJDBCObjectFactory interface belongs in >am, and ClientJDBCObjectFactoryImpl* in net. In order to follow the >naming convention that is already used in the client driver, I would >suggest these class/interface names (or something similar): > > org.apache.derby.client.am.JDBCObjectFactory > org.apache.derby.client.net.NetJDBCObjectFactory > org.apache.derby.client.net.NetJDBCObjectFactory40 > >Additionally, I wonder if the method signatures in the factory >interface are as generic as they should be. There are a lot of newNet* >methods. I thought the whole point with factory methods was to hide >the implementation details. > >For instance, the method newNetConnection() has this signature: > > NetConnection newNetConnection(NetLogWriter netLogWriter, > String databaseName, > java.util.Properties properties) > throws SqlException; > >Wouldn't it be more the Derby way (not that the Derby way necessarily >is the best way...) to rename it to newConnection() and let it have >the following signature: > > Connection newConnection(LogWriter logWriter, > String databaseName, > java.util.Properties properties) > throws SqlException; >? > > >
