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 22:09:06 GMT
Hi Phil,

comments in line again ...

Phil Steitz wrote:

> On 1/14/06, Jörg Schaible <joerg.schaible@gmx.de> wrote:
>> Hi folks,
>>
>> reviewing c-id I found more issues to resolve before releasing 1.0 final.
>> Since I already reported different complaints, I create here a summary of
>> anything already reported and all new issues.
>>
>> 1/ Concept of IdentifierGeneratorFactory
>>
>> The IdentifierGeneratorFactory provides a easy-to-use concept for casual
>> use of id generators. Three elements are part of it:
>>
>> abstract class IdentifierGeneratorFactory - defines the generator types
>> class DefaultIdentifierGeneratorFactory - delivers implementations
>> class IdentifierUtils - easy id generation using the default factory
>>
>> the basic idea is, that a user can implement an own factory and provide
>> it with a system factory and it is automatically used as default in the
>> IdentifierUtils - so far so good.
>>
>> The bad part in the concept is, that we have a variety of generator
>> implementations and all of them are quite different in their
>> initialization. Therefore has the IdentifierGeneratorFactory not only
>> three simple methods to generate the default generator for each
>> (currently supported) id type (String, Long and UUID), but has a variety
>> of methods tailored exactly for the ctors of the different generator
>> implementations, which makes this "factory" quite obscure.
>>
> Agreed.  This is a consequence of the growth in the number of
> different generator types.
> 
>> Modifying a class with every new implementations is for me a kind of
>> "code smell". And the current code already gives evidence of this, since
>> we have new generator implementations, that cannot be
>> accessed/initialized with the factory. Additionally some methods have
>> been added to support newer generator implementations, but the
>> appropriate methods to make use of the generated ids are missing in the
>> IdentifierUtils.
>>
> Agreed again.  The idea was to allow pluggable implemenations of the
> different generator types, but as the number of types increases, this
> gets unweildy.
> 
>> So we either complete those 3 classes for all generator implementations
>> or tailor them to a useful subset for the casual user:
>>
>> interface IdentifierGeneratorFactory {
>>         LongIdentifierGenerator longIDentifierGenerator();
>>         StringIdentifierGenerator stringIDentifierGenerator();
>>         UUIDIdentifierGenerator uuidIDentifierGenerator();
>> }
> 
> But then you lose the distinction among the different subtypes. The 
> question is, do we care about making the implementation of, say
> prefixAlphanumericGenerator pluggable (i.e., would anyone ever really
> want to provide a different factory implementation that returned a
> generator different from PrefixAlphaNumericGenerator from the one we
> provide?  And if so, is the current setup the best way to support
> that.  Answer is likely "no" to at least the second question.

For me the question is, does a normal user really care about the specifics
of an unique id. The most difference is the returned type of the id,
because that may impact other parts of the code (e.g. DB). If we provide a
sensible default for each type, it might be enough. And then it is really
simple for everyone to provide a different generator for each of the types.

>> The question is, if a user *normally* requires in one application
>> different
>> UUID genenerators, string generators or long generators?  In all special
>> cases the generator instance can be generated directly or by an
>> appropriate environment (standard use case in IoC containers, where the
>> implementation and their initialization is configured anyway).
>>
>> Proposal: Use as default a VersionFourGenerator, SessionIdGenerator and
>> LongGenerator. If he has the need for a different generator or a
>> different initialization, he can still provide easily an own factory and
>> set the system property.
> 
> It might actually be better to dispense with
> IdentifierGeneratorFactory altogether, leaving the utils class in
> place, but having it directly instantiate provided impls.

Well - as pico guy - the factory is for me some kind of poor man's IoC
approach, but it might have a place if it is much simpler. But even for the
utils class the argumentation is similar: Do we need a static instance for
every generator type and their different initializations and an accessor
method for each nextIdentifier() method? You have the same problem here:
Add a new generator implementation and you have to add one or more
appropriate methods and static instances to the utils class. Use the "type"
approach from above and you have 3 instances with 3 methods accessing the
specified factory.

> This would 
> also allow us to drop the [discovery] dependency (with some additional
> work on uuid if we decidt to include this in the release - see below).

See also below ...

[snip issues 2-7 for which I hope also to receive some comments one time
<g>]

> One thing that I would like for us to consider is to omit the uuid
> generators from 1.0.  There is a lot to do to get that package into
> releaseable state, including spec compliance review, and I am a little
> concerned with our ability to support it once released, as the
> original developer is no longer working on this code. 

Uuuh! Cough! Bummer! When I look into the answers given to a lot of users
over the last two years, they were always told, c-id is stable and
reliable, we need just to close the minor issues left in Bugzilla and
clean-up the docs. I know quite some projects that use a snapshot of c-id
to generate UUIDs in production (including some of mine). And look into
Bugzilla. We had not much issues about UUID and the last open one can be
easily solved by different implementations for the state. My issue list
targetted this area anyway (concerning Exception handling and the state
problem in this area).

> I would be +1 
> to cleaning up the rest of the package, promoting to proper and
> releasing a simple 1.0 without the uuid generators.  I think the
> package is quite useful without the uuid functions and if there is
> sufficient community interest and support we can add the uuid stuff
> into a 1.1.

Fact is, that UUIDs are part of c-id now for exactly two years and despite
the fact, that c-id had no proper release, they are already used. Even I
cannot use c-id-1.0, if it is without UUID support ... :-/

- 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