Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: neutral (athena.apache.org: local policy)
Message-ID: <4CFE91ED.8010308@free.fr>
Date: Tue, 07 Dec 2010 20:58:37 +0100
From: Luc Maisonobe <Luc.Maisonobe@free.fr>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
 rv:1.9.2.12) Gecko/20101027 Thunderbird/3.1.6
MIME-Version: 1.0
To: Commons Developers List <dev@commons.apache.org>
Subject: Re: svn commit: r1042596 - in
 /commons/proper/math/trunk/src/main/java/org/apache/commons/math:
 analysis/solvers/ distribution/
References: <20101206115626.C2D1D23889E3@eris.apache.org>
 <4CFD39AC.7030309@free.fr>
 <20101206222708.GL16182@dusk.harfang.homelinux.org> <4CFD6515.70102@free.fr>
 <20101207094720.GV11216@dusk.harfang.homelinux.org>
In-Reply-To: <20101207094720.GV11216@dusk.harfang.homelinux.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 8bit

Hi Gilles,

Le 07/12/2010 10:47, Gilles Sadowski a �crit :
> Hello.
> 
>>> [...]
>>>
>>> I think that we can satisfy this use-case without exposing the variables:
>>>
>>> ---CUT---
>>>> BrentSolver mySolver = new BrentSolver();
>>>>
>>>> // initialize GUI
>>>> solverGui.setRelAccuracy(mySolver.getRelativeAccuracy());
>>>> solverGui.setAbsAccuracy(mySolver.getAbsoluteAccuracy());
>>>>
>>>> // fire the GUI
>>>> ...
>>>>
>>>> // in the action callback from the GUI OK button, create the solver
>>>> double rel = solverGui.getRelAccuracy();
>>>> double abs = solverGui.getAbsAccuracy();
>>>>
>>>> mySolver = new BrentSolver(rel, abs);
>>> ---CUT---
>>
>> Creating two solvers to use only one and simply hiding constants seems
>> awkward to me. Just letting the constants visible is simpler.
> 
> You don't use only one. And, arguably, for the GUI, the first solver
> instance is the most important because it provides the needed information.
> 
> What bothers me most with the variables is:
>  1. The redundancy:
>       The value is stored once in the static variable of the class and a
>       second time in each instance.

I don't understand. This is exactly what you have already done. The
current implementation only changed the static variables from public to
private. There are still independent class variables in each class and
independent instant variables in each instance (inherited from the
abstract class).

>  2. The possible inconsistency:
>       Nothing prevents a (sub)class to *not* use the default. So the
>       documentation could be misleading whereas the accessor will always
>       provide the actual value.

Of course a subclass can use something else, this is a main feature of
subclassing: adapting behavior. Also regardless of how the default value
is defined (once publicly in a top level class or several time privately
in each subclass), the user can always choose to build his builder with
non default values. The accessors are not here to provide defaults, they
are here to provide the actual values.

What we are discussing here is not about the actual values that are
stored in the instances. We are discussing about the default constant
values which *may* be used to initialize the actual ones. They are
simple constants, they are currently all the same and they are
duplicated 8 times.

> 
> GUIs _are_ awkward. Having to create two solvers in such a context is
> negligible and a very small price to pay for ensuring consistency.
> 
>> I think what Sebb pointed out in this issue was not that the constants
>> were public, but that these constants were duplicated. I agree with him
>> here. However solving the issue by removing the duplication and having
>> constants defined in the top level interface is more straightforward.
> 
> There are 2 problems with this:
>  1. The top-level class where such "public" constants should be defined is
>     "BaseAbstractUnivariateRealSolver". This is not a "user" class but a
>     "developer" class, an implementation detail. Encouraging users to have
>     that class appear in their code is not good design.

Then we can put them in the UnivariateRealSolver interface. Interfaces
can have constants and are a great place to store them in fact.

>  2. The "accuracies" might mean different things for different solvers and
>     thus the defaults might be set to different values depending on the
>     implementation. [That is why I had created a static variable for each
>     class.]

If a specific implementation needs a different value, then it can use
its own default, either by having its own constant (it cannot reuse the
name from the top interface here).  However, for now it is clearly not
the case. There are 8 implementations and 8 times the exact same value
(1.0e-6).

> 
> Please note also that by using the accessor, users can code through
> interface while with the static constant, their code must reference a
> concrete class.

No, an interface can define a constant.

best regards,
Luc

> 
> 
> Best,
> Gilles
> 
> ---------------------------------------------------------------------
> 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