On 11/4/05, Martin van den Bemt <[EMAIL PROTECTED]> wrote:

> Just was running the tests, and the test are failing (you probably noticed).
> The main thing is that it is failing on getAutoIncrementColumn().
>
> The following issues come to mind :
>
> - Rename getAutoIncrementColumn() to getAutoIncrementColumns() for clarity.

+1

> - There are assertNull checks, but the method never returns null, it will at 
> least return an empty
> column array. Does this need to return null, or should I change the test to 
> expect an empty array ?
> (I am guessing the latter since current code has no null checks)
> Also javadoc needs to mention what it returns when no columns are there.

Empty array is better, makes using it simpler (no need to test for null).

> - Maybe we need to add a shortcut for getAutoIncrementColumn(), since (most 
> if not a lot of)
> databases only allow one increment column anyway (we can just add a note to 
> the javadoc that it is
> safer to use getAutoIncrementColumns()).

Actually, I think every database that supports sequences automatically
supports multiple auto-increment columns (you can always add the
unique constraints manually).
And what should the shortcut do if there is more than one ? Throw an exception ?

> So will be doing some work on DatabaseIO and DatabaseIOTest..

Cool. It would be really great if you could have a look at the issues
that are related to the jdbc model reader (I've added you to the
ddlutils-developer JIRA group, so you can edit DdlUtils' issues).

> BTW can you send a mail to [EMAIL PROTECTED] ? (Unless you
> aren't a moderator, than it doesn't do anything..). This way my commit mails 
> come through to the
> mailinglist.

I'm not yet, but I asked the PMC.

> Will be checking in again with you if I am not sure what you had in mind..

Ok! Unfortunately I'm busy 'til the 18th because I have to prepare my
tutorial for ApacheCon US, so I cannot be of much help, only a bit
here and there.

Tom

Reply via email to