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. Oh well, I'll try and find a way. Thanks anyway! Sébastien
>> I'm currently puzzled by the SymmLQ class. I feel the need for some >> cleaning up, but I do not really know where to start. So if anyone >> feels like reviewing the code, I would be grateful for some help. The >> good news is that unit tests are very extensive, so we can try drastic >> refactoring. >> >> Thanks for your help! >> Sébastien >> >> [1] Which is probably ill-named. Should be named "cleaning-up SymmLQ" >> [2] I have to say I have mixed feelings now: I'm wondering whether a >> 600-hundred lines method would not be better in that case, but I would >> probably not get a wide support... >> >> >> --------------------------------------------------------------------- >> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >> For additional commands, e-mail: dev-h...@commons.apache.org >> > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org