commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jörg Schaible <joerg.schai...@gmx.de>
Subject Re: [id] Review before 1.0 (Summary)
Date Sat, 14 Jan 2006 23:26:27 GMT
Hi fellows,

two of the issues have already be solved (see inline comments), but I have
two new ones, see at the bottom.

Jörg Schaible wrote:

[snip]

> 3/ xxxStateImpl classes
> 
> The javadoc of ReadOnlyResourceStateImpl implies that the implementation
> tries to discover (well, use c-discovery) to load a file "uuid-[n].conf".
> In reality it expects a system property (wrong name documented) set to a
> file name that can be loaded as resource. All unit tests use a file
> "uuid1.state", that is only available during the tests.
> 
> I am just wondering, if a default is simply missing or left by intension.

This one is answered by the UUID doc of c-id.
 
> Also the derived ReadWriteFileStateImpl tries to write into the URL of the
> resource ... which will fail badly if the file is within a jar.

This part is addressed in the only current open Bugzilla issue with a
comment from Tim.

[snip]

> 5/ Unit tests
> 
> A lot of unit tests in the c-id package have own suite methods and set an
> own name for the test. While there are situations, where the usage of a
> suite can be necessary, here it is definitely not. This not only results
> in an inconsistent unit test report, but also prevents, that e.g. in
> Eclipse I can simply double click the test to get to the appropriate code.
> 
> Proposal: Remove unnecessary suite methods and TestCase ctors taking a
> name.

Phil agreed on this in private mail. Done.

[snip]


8/ Prefix generators

We have 3 generators, that add a prefix to the generated id. All 3 classes
to mainly the same for 3 different StringIdentifierGenerators.

Proposal: Since we have a lot more StringIdentifierGenrators (e.g. the
UUIDIdentifierGenerators a StringIdentifiers too), I would refactore this 3
classes into a wrapper in a separate package o.a.c.id.wrapper and delegate
to an arbitrary StringIdentifierGenerator implementation:

class PrefixStringIdentifierGenerator implements StringIdentifierGenerator {
        PrefixStringIdentifierGenerator(StringIdentifierGenerator delegee) {
                this.delegee = delegee;
        }
        ...
}

9/ Wrappable genrators

The generators in the o.a.id.serial package generate ids in a sequence. A
lot of them can start over if the maximum value is reached based on the
"wrap" flag.

Proposal: Introduce a Wrappable interface, that is already implemented by
those ones:

interface Wrappable {
        boolean isWrapping();
        void setWrap(boolean wrap);
}



Comments?

- Jörg


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message