commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [Math] Cleanup in "HarmonicFitter" and "GaussianFitter"
Date Fri, 17 Aug 2012 11:15:53 GMT
On Fri, Aug 17, 2012 at 11:46:22AM +0100, sebb wrote:
> On 17 August 2012 11:24, Gilles Sadowski <gilles@harfang.homelinux.org> wrote:
> > On Fri, Aug 17, 2012 at 09:40:49AM +0100, sebb wrote:
> >> On 17 August 2012 07:40, Luc Maisonobe <Luc.Maisonobe@free.fr> wrote:
> >> > Le 16/08/2012 23:48, sebb a écrit :
> >> >> On 16 August 2012 21:58, Gilles Sadowski <gilles@harfang.homelinux.org>
wrote:
> >> >>> Hello.
> >> >>>
> >> >>> In classes "HarmonicFitter" and "GaussianFitter" (in package
> >> >>> "o.a.c.m.optimization.fitting"), there is an inner class "ParameterGuesser".
> >> >>> All the necessary input for the guessing procedure is passed to
the
> >> >>> constructor; I thus propose that the guessing is performed at object
> >> >>> construction rather than at the call to the "guess()" method. This
will
> >> >>> allow to declare all the fields "final". Moreover, all that can
go wrong
> >> >>> will be located in the constructor, turning the "guess" method
into a mere
> >> >>> accessor.
> >> >
> >> > +1 to Gilles suggestion.
> >> >
> >> >>
> >> >> On the face of it, it sounds like the class does not even need to be
> >> >> instantiated.
> >> >> Could the guess method be turned into a static utility method to which
> >> >> the parameters are passed?
> >> >> Why bother creating an instance only to return the guess later?
> >> >
> >> > It could be. There are a bunch of methods involved and they exchange
> >> > data which is now stored in a few class fields (observation arrays and
> >> > the three estimated parameters), so these data should be added as
> >> > parameters to the static functions.
> >> >
> >> > I'm not sure whether it is cleaner to have a small instance created for
> >> > a short time to perform some difficult preprocessing or to have a bunch
> >> > of static methods.
> >>
> >> Instances need to be disposed of by GC whereas parameters
> >> automatically disappear when a called procedure returns.
> >
> > As a short-lived object, I guess that performance will not be improved
> > greatly (especially w.r.t. the time taken for the fitting itself).
> >
> >> A class with no fields is guaranteed immutable.
> >
> > The class will be immutable.
> >
> >>
> >> But for large numbers of parameters, class data is perhaps cleaner.
> >
> > Being "public", the inner class cannot disappear now; so I'll do the first
> > cleanup, and we'll see later if further changes are in order.
> 
> Is the class directly usable by 3rd parties?

Yes.

> Would it make sense for a 3rd party to use it directly?

Maybe. They could compare the guessed values with the optimized ones.
Admittingly, this is not very useful... if everything works fine.

> 
> If not, then although it would break strict binary compat., it would
> be possible to consider changing it.
> If it's not directly used by 3rd party code, I don't think changes can
> break their code.

I don't understand: the class is public. Someone can have instantiated it in
his code...

> 
> Ideally any such helper classes should be private or package protected
> ab initio.

I agree.
However in this case, the boundary between helper and utility (that could be
useful on its own) is not clear-cut.


Gilles

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


Mime
View raw message