[ 
https://issues.apache.org/jira/browse/MATH-512?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12996336#comment-12996336
 ] 

Gilles commented on MATH-512:
-----------------------------

{quote}
I already submitted the second patch earlier today.
{quote}

I know, and I looked at it; but it still contains many formatting errors ("tab" 
characters, trailing white space, "{" characters on their own line).

The first two kinds of errors are detected by "CheckStyle"; you can generate 
the report by running the following command:

{noformat}
  $ mvn site
{noformat}

and the html report will be located in the file

{noformat}
  target/site/checkstyle.html
{noformat}

For the third kind, you'll probably have to review the code manually.

I think that it is better to correct as many problems as possible before 
committing any changes. Even so, some will remain, but the fewer, the better, 
especially for those who clean-up afterwards ;-).

[By the way, I think that the name of the inner class 
"GaussianParametersGuesser" could be shortened to maybe "ParameterGuesser".]

> "GaussianFitter" could inherit from "CurveFitter"
> -------------------------------------------------
>
>                 Key: MATH-512
>                 URL: https://issues.apache.org/jira/browse/MATH-512
>             Project: Commons Math
>          Issue Type: Improvement
>            Reporter: Gilles
>            Priority: Minor
>             Fix For: 3.0
>
>         Attachments: GaussianFitter.patch, GaussianFitter.patch, 
> GaussianFitterTest.patch, ParametricGaussianFunctionTest.patch
>
>
> Currently, the "GaussianFitter" class contains a "CurveFitter" instance (and 
> delegates most method calls to it) for the sake of being able to return a 
> "GaussianFunction" instance (from the call to the "fit" method).
> I think that it would clearer (in the context of the "fitting" package) that 
> "fit" returns an array of doubles (as "CurveFitter" does). The user can then 
> easily create a "Gaussian" function object, if needed.
> [Also, note that the "GaussianFitter" actually fits the sum of a constant and 
> a Gaussian function; thus the name is slightly misleading IMO.]
> I also think that "ParametricGaussianFunction" and 
> "GaussianParametersGuesser" should become inner classes of "GaussianFitter" 
> (as this makes it clear what their use is).

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to