commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: [math] Checkstyle too stringent?
Date Mon, 19 Mar 2012 08:55:36 GMT
2012/3/19 Sébastien Brisard <sebastien.brisard@m4x.org>:
> 2012/3/18 sebb <sebbaz@gmail.com>:
>> 2012/3/18 Sébastien Brisard <sebastien.brisard@m4x.org>:
>>> Hi,
>>>
>>> 2012/3/18 sebb <sebbaz@gmail.com>:
>>>> 2012/3/18 Sébastien Brisard <sebastien.brisard@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.

For some reason Eclipse showed it as unused by the parent class at one
point, but that is not the case.

> Sébastien
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message