Le 19 mars 2012 07:11, Sébastien Brisard <sebastien.bris...@m4x.org> a écrit : > 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. > Oh! You've done that already! I'll just modify the parameter order in the constructor of State, to be consistent with the constructor of SymmLQ.
>> 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
