Martin,
if you think about submitting a patch as indicated in 2), I think it would be
worthwhile to rename the transactionManager property to something like
transactionManagerName. At the moment, the way it is implemented assumes that the
javax.jta.TransactionManager instance of a j2ee container can be looked up via JNDI
... which is valid for BEA's WebLogic and JBoss, but not for others such as WebSphere
4.0.
In addition, let me just say that I am very much in favour of most of the things you
said, e.g. increased use of interfaces, decoupling interfaces from their (default)
implementations, etc. Similar things are currently being discussed under a different
thread as well, focusing on the use of JMX for modularisation of Castor and (possibly
maybe) and Avalon as a component framework.
Thanks
Werner
Martin Renner wrote:
> Hi.
>
> I am implementing an enhancement to Castor JDO. Right now, this enhancement is
> very specific, so it is not worth to discuss about the enhancement itself. For
> my enhancement, I have to supply my own implementation of TransactionContextImpl.
>
> (1)
>
> The design of
> interface Database
> class DatabaseImpl implements Database
> and
> abstract class TransactionContext (for the generic part)
> class TransactionContextImpl extends TransactionContext (db specific stuff)
> is nice, but not really useful right now.
>
> JDO.java is creating (hardcoded) an instance of DatabaseImpl. DatabaseImpl is
> creating (hardcoced) an instance of TransactionContextImpl. So there is no way
> to plug-in my specific TransactionContextImpl, except of subclassing "JDO" and
> "DatabaseImpl" and overriding "JDO.getDatabase()" and "Database.
>
> If there is already a generic approach with interfaces and abstract base
> classes, then there should be also a generic (and pluggable) approach to create
> specific instances. What do you think about this?
>
> (2)
>
> Subclassing of DatabaseImpl is almost impossible, because all attributes are
> private and some "getters" are missing. I added three protected getters for
> "lockTimeout", "autoStore" and "CallbackInterceptor".
>
> If the Castor team agrees, I can post the corresponding diffs.
>
> (3)
>
> DatabaseRegistry.loadDatabase is loading the mapping AND is checking for a valid
> database connection (DataSource directly, DataSource via JNDI, Driver). IMHO,
> the checking for a valid database should be decoupled from the loading and
> checking of the type mapping (one reason: database.xml and mapping.xml are
> different files). Perhaps to a protected method?
>
> (4)
>
> DatabaseRegistry has a method "java.sql.Connection createConnection()". The only
> usage of this method is in "SQLEngine.getSeparateConnection()". Again
> decoupling: Interface Persistence has no java.sql.Connection, but SQLEngine
> (which implements Persistence) has java.sql.Connection's. And DatabaseRegistry
> is coupled to SQLEngine (or vice versa). What about a DatabaseRegistry without a
> physical java.sql.Connection. What about different implementations of
> Persistence (besides SQLEngine)? I think, that SQLEngine should get the ability
> to create a separate connection. It should not use DatabaseRegistry for this.
>
> (5)
>
> TransactionContext contains
> private final javax.transaction.xa.Xid _xid;
> and a constructor
> public TransactionContext( Database db, Xid xid )
>
> However, "_xid" is never used. Also, this constructor is never used.
>
> To make things clearer, I would remove this constructor, "_xid" and two
> confusing import statements from the source code.
>
> I can supply diffs for (2) and (5) and I would be pleased to get some feedback
> for the other items.
>
> Martin
>
> -----------------------------------------------------------
> If you wish to unsubscribe from this mailing, send mail to
> [EMAIL PROTECTED] with a subject of:
> unsubscribe castor-dev
-----------------------------------------------------------
If you wish to unsubscribe from this mailing, send mail to
[EMAIL PROTECTED] with a subject of:
unsubscribe castor-dev
