Michael Hunger wrote: > 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).
Good stuff, I've added the code to my local checkout. I'm refactoring lots of other related code right now, and will commit it along with that. > 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. Annotations can't have methods I think, and neither the models nor the annotations should have this factory code becuase they are in API/SPI and the factory code is in runtime. I don't think this particular factory has references to runtime methods, but I know there is other factory that does, so rather then spread out the factory code in API, SPI and runtime it is being concentrated in runtime. > 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. Yeah, I would say that the code you have been refactoring was work in progress, hence the confusion. The refactoring you submitted was a good step to get it more readable though, and the same needs to be done for a lot of the other factory code which right now is decidedly messy. I'm working on that right now. /Rickard _______________________________________________ qi4j-dev mailing list [email protected] http://lists.ops4j.org/mailman/listinfo/qi4j-dev
