You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Sebb (JIRA)" <ji...@apache.org> on 2011/02/05 14:26:30 UTC

[jira] Issue Comment Edited: (MATH-506) The static field ChiSquareTestImpl.distribution serves no purpose

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

Sebb edited comment on MATH-506 at 2/5/11 1:24 PM:
---------------------------------------------------

Just tried removing the field and setter in 3.0, and found that the constructors rely on the setter (which is a separate bug, as the setter is not final - but easily fixable if required).

The fix for MATH-349 merely removed deprecated code.

It replaced "distribution.setDegreesOfFreedom(dof)" with "distribution = new ChiSquaredDistributionImpl(dof)" which is how the field became useless.

There are two constructors which still create values for the distribution field.

I don't know enough about the Math to know whether there would be any use cases for having additional methods that used a distribution provided by the class instance, rather than calculated by the individual methods (as at present).

If there is no need for external provision of the distribution degree of freedom, then the constructor with parameter can be dropped.

Otherwise, we need to add some methods that can use the provided distribution (which should be a final instance field).

In any case, I think the setter needs to be dropped from 3.x

      was (Author: sebb@apache.org):
    Just tried removing the field and setter in 3.0, and found that the constructors rely on the setter (which is a separate bug, as the setter is not final - but easily fixable if required).

The fix for MATH-349 merely removed deprecated code.

It replaced "distribution.setDegreesOfFreedom(x)" with "distribution = new ChiSquaredDistributionImpl(x)" which is how the field became useless.

There are two constructors which still create values for the distribution field.

I don't know enough about the Math to know whether there would be any use cases for having additional methods that used a distribution provided by the class instance, rather than calculated by the individual methods (as at present).

If there is no need for external provision of the distribution degree of freedom, then the constructor with parameter can be dropped.

Otherwise, we need to add some methods that can use the provided distribution (which should be a final instance field).

In any case, I think the setter needs to be dropped from 3.x
  
> The static field ChiSquareTestImpl.distribution serves no purpose
> -----------------------------------------------------------------
>
>                 Key: MATH-506
>                 URL: https://issues.apache.org/jira/browse/MATH-506
>             Project: Commons Math
>          Issue Type: Bug
>    Affects Versions: 3.0
>            Reporter: Sebb
>            Priority: Minor
>
> The static field ChiSquareTestImpl.distribution serves no purpose.
> There is a setter for it, but in every case where the field is used, it is first overwritten with a new value.
> The field and the setter should be removed, and the methods that create a new instance should create a local variable instead.
> For Math 2.1, the field can be removed and the setter deprecated.

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