Hi Werner,

See inline.


>(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?

The idea is that you can have your own JDO implementation.
Those pieces of code are designed to be generic. However,
it is not designed that you can replace one or two classes, but
whole layer. For example, the early design goal is to support
different set of API, like ODMG, JDO, EJB, and DAX at the
same time. (see http://castor.exolab.org/design-persist.html).

>(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.

See my reply under (1)

>(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?

But, I am not sure about this. If I have time, I take
a look of the code.

>(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)?

We used to have a LDAP engine. That's doesn't use DatabaseRegistry.
You may dig to 0.8.x version if you are very interested.

>I think, that SQLEngine should get the ability
>to create a separate connection. It should not use DatabaseRegistry for
this.

I trend to agree on this. However, it is somehow more complicated
that it appear. Because, for simple JDBC connection, we initialize
"commit" on the connection. For XA, the transaction manager
is the one who do it.

SQLEngine is designed to be stateless. And, we want to have
the transaction code in higher level.

>(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 think it is a left over when we remove the XAResourceImpl.

>I can supply diffs for (2) and (5) and I would be pleased to get some
feedback
>for the other items.

I am not sure I agree with (2). But, a diff to (5) would be welcome.


Thanks,


Thomas

----------------------------------------------------------- 
If you wish to unsubscribe from this mailing, send mail to
[EMAIL PROTECTED] with a subject of:
        unsubscribe castor-dev

Reply via email to