Fwd: Re: [kaffe] java.sql.*

[Dalibor Topic]

hi Ito,

--- Ito Kazumitsu <ito.kazumitsu@hitachi-cable.co.jp>
wrote:

> This is the results of my review:
> 
> 1. Exceptions only GNU Classpath's java.sql throws
> 
>    Omitted (too many to show here).  They are all
>    SQLException. Most of them are supposed to be
> thrown by
>    methods added since 1.2,  which kaffe does not
> implement.

that's o.k.
 
> 2. Exceptions only kaffe's java.sql throws
> 
> Date:    public static java.sql.Date
> valueOf(java.lang.String) throws
> java.lang.IllegalArgumentException
> DriverManager:    public static synchronized
> java.sql.Connection getConnection(java.lang.String)
> throws java.sql.SQLException
> DriverManager:    public static synchronized
> java.sql.Connection getConnection(java.lang.String,
> java.lang.String, java.lang.String) throws
> java.sql.SQLException
> DriverManager:    public static synchronized
> java.sql.Connection getConnection(java.lang.String,
> java.util.Properties) throws java.sql.SQLException
> DriverManager:    public static synchronized void
> registerDriver(java.sql.Driver) throws
> java.sql.SQLException
> Time:    public static java.sql.Time
> valueOf(java.lang.String) throws
> java.lang.IllegalArgumentException
> Timestamp:    public static java.sql.Timestamp
> valueOf(java.lang.String) throws
> java.lang.IllegalArgumentException
> 
>   Kaffe's Date and Time throwing
> IllegalArgumentException is not
>   conforming to Sun's API document.

so that would be fixed by the switch.
 
>   GNU classpath's Timestamp not throwing
> IllegalArgumentException
>   is not conforming to Sun's API document.

mhm, that's not very serious, I think, given it's a
RuntimeException and as such doesn't have to be
specified in the throws clause.
 
>   As for DriverManager, they are listed here because
> of the
>   differrnce of getConnection methods.  Kaffe's
> declare synchronized
>   but GNU classpath's does not.

looking at getConnection from classpath, I don't think
there is a reason for it to be synchronized, as it
only touches the current state of the driver manager
once through getDriver(). 

They should sync getDriver/s though. But looking at
kaffe's code also shows a number of synchronization
problems, so I'd say let's merge in Classpath's
version and fix it.
 
> 3. Differnce of SQLException
> 
>   The following methods are quite different.
> 
> Kaffe's
> public synchronized void
> setNextException(SQLException ex) {
>         if (next == null) {
>                 next = ex;
>         }
>         else {
>                 next.setNextException(ex);
>         }
> }
> 
> GNU Classpath's
>   public void setNextException(SQLException e)
>   {
>     if (e == null)
>       return;
> 
>     SQLException list_entry = this;
>     while (list_entry.getNextException() != null)
>       list_entry = list_entry.getNextException();
> 
>     list_entry.next = e;
>   }

I don't think they are as different as it seems. Both
append the exception to the end of the exception
chain. Kaffe does it using recursion, Classpath does
it using a loop. 

Classpath also has a shortcut when e is null to abort
looping through it. So if e is null, kaffe would
search till the end of list, and set next (which is
already null) to null, wehere classpath returns
instantly. A class invariant is that the next
exception of the last exception in the chain is null,
thus there is no way to 'overwrite' an exception by
null. So while both approaches are correct,
classpath's is faster.

so I went ahead and merged in the interfaces, the
exceptions and most of the classes from Classpath in
the CVS.

I've left out 4 classes for now, because they have
some more interesting differences, listed below.

Date, Time :

missing get methods.
-> uses to be a problem with reflection, may be a
problem with classes compiled with one version not
working with the other.

Date, Time, Timestamp :

valueOf: unclear how it handles illegal arguments.
kaffe says throw exception, classpath says return
null.
-> what does JDK do?

DriverManager:

getDriver: doesn't throw an SQLException if no driver
was found. kaffe does.
-> what does JDK do?

Could you write testcases DriverManager & Date and see
how Sun's JDK
behaves ? I'll take care of the missing get methods.

cheers,
dalibor topic

__________________________________________________
Do you Yahoo!?
Yahoo! Tax Center - File online, calculators, forms, and more
http://tax.yahoo.com