2012/3/18 sebb <seb...@gmail.com>: > 2012/3/18 Sébastien Brisard <sebastien.bris...@m4x.org>: >> Hi, >> >> 2012/3/18 sebb <seb...@gmail.com>: >>> 2012/3/18 Sébastien Brisard <sebastien.bris...@m4x.org>: >>>> Hi, >>>> I'm trying to clean up the SymmLQ class (see MATH-761 [1]). >>>> The problem is the heart of the algorithm is very long (it would >>>> probably amount to more thatn 600 lines). When I first implemented it, >>>> I figured that I could split this algorithm in several methods: >>>> init(), update(), updateNorms(), refine(), etc... This makes the code >>>> somewhat more legible [2], with the difficulty that there is a lot of >>>> data to be shared between these methods. That's the reason why I >>>> created a simple nested, container class (namely, State) which holds >>>> all the variables which are updated at each iteration. In order to >>>> make checkstyle happy, all the fields of this nested class are >>>> private, which means that a lot of accessors should be created with a >>>> null benefit, since this class is nested anyway. I have to admit I >>>> forgot to create these accessors, so synthetic accessors are >>>> automatically generated at the present time. I'm not sure this is good >>>> practice, but I'm also reluctant to implement all those (useless in my >>>> view) accessors. So my question is two fold >>>> >>>> 1. Would it be acceptable to have public fields in a nested class? If >>>> yes, the checkstyle rules should be modified. >>>> 2. Is it good practice otherwise to let the JVM generate synthetic >>>> accessors (which would probably get inlined anyway by modern JVMs?). >>> >>> Why not put the methods and the data in the same class? >>> >>> That would eliminate the need for accessors. >>> >> That's actually what I've done: init(), update(), refine() and the >> likes all belong to the class State. This indeed reduces the problem, >> but does not remove it completely. > > As far as I can tell, you can make the State class static with very > little effort. > Just pass in the (final) fields check and delta when creating the > class (and store in State final fields). > Thanks for your review! I'll try something along these lines. To tell the truth, I've tried that already, but was not too pleased with the result. I can't remember why, though. You're right that making State a static class would be an good first step.
> The field delta is not otherwise used by the main class so can perhaps > be dropped as an instance field there. > delta is inherent to the solver itself (required accuracy), not to the linear system to be solved. I know people might disagree with that, but I think that it should be kept as an instance field. Sébastien --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
