I refactored the stuff to make it more readable and expressive.
Problems were (to broad scope, lazy initialization, missing final,
violation of law of demeter, naming, long method, cascaded loops).
There are still violations of the law of demeter, especially around
those collections. Either you have the annotations have FactoryMethods
to create the models needed or you add the creation factory methods to
the models themselves.
Now you can clearer see that the ConstraintDeclarationModel is not used.
But also that your notion was wrong as the ConstraintModel and the List
are only created once and then added to (if (list != null) ) I joined
declaration and creation as the lazy init seems unneccessary.
if you look at ConstraintDeclarationModel this is an almost empty anemic
model with a big "TODO" comment at the top.
Have fun,
Michael
The curly brackets formatting is quite unusual for Java code.
protected PropertyModel getPropertyModel( final Method method )
{
final List<Annotation> constraintAnnotations =
getAnnotationsForType( method, ConstraintDeclaration.class );
for( final Annotation constraintDeclAnnotation :
constraintAnnotations )
{
createConstraintDeclarationModel(
constraintDeclAnnotation.annotationType() );
}
return new PropertyModel( method, new ConstraintsModel(
constraintAnnotations ) );
}
private void createConstraintDeclarationModel( final Class<?
extends Annotation> constraintDeclAnnotationType )
{
final List<ConstraintModel<?>> constraintModels =
getConstraintModels( constraintDeclAnnotationType );
final ConstraintDeclarationModel constraintDeclarationModel =
new ConstraintDeclarationModel(
constraintDeclAnnotationType, constraintModels );
}
private List<ConstraintModel<?>> getConstraintModels( final Class<?
extends Annotation> constraintDeclAnnotation )
{
final Constraints constraintsAnnotation =
constraintDeclAnnotation.getAnnotation( Constraints.class );
if( constraintsAnnotation == null )
{
return Collections.emptyList();
}
final List<ConstraintModel<?>> constraintModels = new
ArrayList<ConstraintModel<?>>();
for( final Class<? extends Constraint<?,?>>
constraintImplementation : constraintsAnnotation.value() )
{
constraintModels.add( new ConstraintModel(
constraintImplementation, constraintDeclAnnotation ) );
}
return constraintModels;
}
private List<Annotation> getAnnotationsForType( final Method
method, final Class<? extends Annotation> annotationTypeClass ) {
final List<Annotation> result = new ArrayList<Annotation>();
for( final Annotation annotation : method.getAnnotations() )
{
final Class<? extends Annotation> annotationType =
annotation.annotationType();
if (null != annotationType.getAnnotation(
annotationTypeClass )) {
result.add(annotation);
}
}
return result;
}
Niclas Hedhman schrieb:
> Actually, looking at the entire method, and more weird stuff shows up;
>
> protected PropertyModel getPropertyModel( Method method )
> {
> PropertyModel propertyModel;
> Annotation[] annotations = method.getAnnotations();
> List<Annotation> constraintAnnotations = null;
> ConstraintsModel constraintsModel = null;
> for( Annotation annotation : annotations )
> {
> if( annotation.annotationType().getAnnotation(
> ConstraintDeclaration.class ) != null )
> {
> Constraints constraintsAnnotation =
> annotation.annotationType().getAnnotation( Constraints.class );
> List<ConstraintModel> constraintModels;
> if( constraintsAnnotation != null )
> {
> constraintModels = new ArrayList<ConstraintModel>();
> Class<? extends Constraint<?,?>>[]
> constraintImplementations = constraintsAnnotation.value();
> for( Class<? extends Constraint<?,?>>
> constraintImplementation : constraintImplementations )
> {
> Class<? extends Annotation> annotationType =
> annotation.annotationType();
> ConstraintModel constraintModel = new
> ConstraintModel( constraintImplementation, annotationType );
> constraintModels.add( constraintModel );
> }
> }
> // This line is never used...
> // ConstraintDeclarationModel
> constraintDeclarationModel = new ConstraintDeclarationModel(
> annotation.annotationType(), constraintModels );
>
> if( constraintAnnotations == null )
> {
> constraintAnnotations = new ArrayList<Annotation>();
> constraintsModel = new ConstraintsModel(
> constraintAnnotations );
> }
> constraintAnnotations.add( annotation );
> }
> }
> propertyModel = new PropertyModel( method, constraintsModel );
> return propertyModel;
> }
>
> The
>
> List<ConstraintModel> constraintModels;
>
> will be populated, but never used.
>
> The
> constraintsModel = new ConstraintsModel(
> constraintAnnotations );
> is sitting in a loop, and only the last one found is used outside the loop.
>
>
> Something is not right here...
>
> Rickard?
>
>
> Cheers
> Niclas
>
> _______________________________________________
> qi4j-dev mailing list
> [email protected]
> http://lists.ops4j.org/mailman/listinfo/qi4j-dev
_______________________________________________
qi4j-dev mailing list
[email protected]
http://lists.ops4j.org/mailman/listinfo/qi4j-dev
