Thanks, yes, the problem is the separation of the modules and which code belongs where (cyclic package/module dependencies). But loading a single Factory class with that many specific methods doesn't seem right either. Perhaps moving the Model-Creation methods to a separate entity would help.
What do you think about these Helper/Utils classes like AnnotationUtils? I don't like repeated code as the problem of keeping it all in sync should not be underestimated. Besides using create Factory Methods instead of constructors gives you a lot more freedom of the implementation and of the readable naming of the method. I looked at the usages of ParameterModel and most of the usages are in a form of Collection, List, Iterable<ParameterModel> I'd like to refactor it to a separate entity named ParameterModels (although the readability of this name .i.e. difference between ParameterModel and ParameterModel_s_ is quite bad)). What about the general usage of the final modifier? I'd like to use it for immutable instance fields, parameters and local variables. Not only it helps to escape typing errors when assigning the variable twice. It also documents the immutability of the target. And it supports a more functional style of command/query separation. What about java-doc documenting the SPI-classes? (I'd do it for the AbstractModelFactory). Michael Rickard Öberg schrieb: > 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 -- Michael Hunger Independent Consultant Web: http://www.jexp.de Email: [EMAIL PROTECTED] Enthusiastic Evangelist for Better Software Development We support Software Engineering Radio (www.se-radio.net) _______________________________________________ qi4j-dev mailing list [email protected] http://lists.ops4j.org/mailman/listinfo/qi4j-dev
