On 24/01/2010, Simone Tripodi <[email protected]> wrote:
> Hi all guys,
> I'm writing you because I really need your help on graduating my effort
> spent on sandbox about the digester: I finally terminated porting my old
> stuff[1] on the sandbox and implemented some test cases that show how to use
> annotations on pojos.
> I need your help on:
> - reviewing the design;
> - make the code more compliant to commons-digester;
> - increasing test cases
> - suggest me what I have to do to propose the sandbox merged in /trunk
> - help me on writing the documentation, I'm not a native English speaker :(
> :(
>
> Is anyone available to help me? I'll start writing an architectural document
> to provide you better informations, and apologize in advance for my poor way
> to explain concepts in English :P
>
> In the meanwhile, if you have spare time to have a look at the code you can
> check it out from svn sandbox[2]
I cannot comment on the design or behaviour, as I'm not familiar with Digester.
However, I can comment on the coding style.
There are a few missing @Override markers.
@SuppressWarnings("unchecked") should be used as close as possible to
the statement that causes the warning, and there should be a comment
as to why the warning can be ignored.
For example, in DefaultLoaderHandler.handle(), the annotation should
be used on the first statement of the try{} block. Similarly for
.MethodHandler.doHandle(), etc.
There are a few private methods where the Javadoc is incomplete, e.g.
MethodHandler.doHandle(). Either drop the Javadoc, or have something
useful there.
The Javadoc for MethodArgument() references a non-existent parameter;
there are a few other Javadoc problems, e.g. the @see references.
It would be useful to document which classes are intended to be
thread-safe, and which are definitely not intended to be thread-safe .
Could the AnnotationRuleProvider.init() method be dropped in favour of
providing the parameters in the constructors? That would allow the
fields to be made final, which would improve thread-safety.
In the MethodArgument class, two of the methods return a pointer to
the final Annotations array. This breaks containment, as it allows
external classes to modify the array. It would be safer to return a
copy of the array. This would also make the class immutable and
therefore thread-safe.
Findbugs warns that the field FromAnnotationsRuleSet.namespaceURI is
never written.
This must be a mistake.
Having said all that, I think the general style seems better than the
existing Digester code, which has a lot of problems, e.g. public
static fields that are not final! [Any such issues should be fixed in
trunk, not here].
By the way - did you need to make any changes to the existing Digester code?
> Thanks in advance, all the best!!!
> Simo
>
> [1] http://code.google.com/p/digester-annotations/ - still on google code,
> do I have to drop it?
> [2] https://svn.apache.org/repos/asf/commons/sandbox/at-digester/trunk
>
> http://people.apache.org/~simonetripodi/
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
