commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sébastien Brisard <sebastien.bris...@m4x.org>
Subject Re: [math] Checkstyle too stringent?
Date Mon, 19 Mar 2012 06:11:27 GMT
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.

Sébastien


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


Mime
View raw message