commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mikkel Meyer Andersen <m...@mikl.dk>
Subject Re: [math] Problem with immutable distribution
Date Wed, 29 Sep 2010 11:57:06 GMT
Hi,

Just to be sure: You propose to remove
public void setDistribution(TDistribution value)
Is this correct?

If so, another proposal - I'm not sure it's a clever one - is

---CUT---
    public TDistribution setDistribution(TDistribution value) {
        double n = value.getDegreesOfFreedom();
        if (n > 2) {
            n -= 2;
        }

        final TDistribution newDistribution = new TDistributionImpl(n - 2);
        distribution = newDistribution;
        return newDistribution;
    }
---CUT---

So that the new instance gets returned. As mentioned, I'm not sure
it's a good idea, but it would maintain the possibility to change
distribution. I think I'd prefer deleting the setDistribution-method
instead of this psudo-solution.

TTestImpl: Would it be an idea to change the constructor to pass a
double degreeOfFreedom instead of a TDistribution?

Cheers, Mikkel.

2010/9/29 Gilles Sadowski <gilles@harfang.homelinux.org>:
> Hi.
>
> While making the distribution classes immutable, I'm encountering a problem.
> In class "SimpleRegression" (package "stat.regression"), there is (at line
> 140):
> ---CUT---
>        if (n > 2) {
>            distribution.setDegreesOfFreedom(n - 2);
>        }
> ---CUT---
> and, similarly, in class "TTestImpl" (package "stat.inference"), there is
> (at line 961):
> ---CUT---
>        distribution.setDegreesOfFreedom(n - 1);
> ---CUT---
>
> The method "setDegreesOfFreedom" does not exist anymore (it was depreceated
> in 2.1).
> I would have created a new instance e.g. for the first example above:
> ---CUT---
>        if (n > 2) {
>            distribution = new TDistributionImpl(n - 2);
>        }
> ---CUT---
> but the problem is the existence of a setter for the "distribution" instance
> variable:
> ---CUT---
>    public void setDistribution(TDistribution value) {
>        distribution = value;
>
>        // modify degrees of freedom
>        if (n > 2) {
>            distribution.setDegreesOfFreedom(n - 2);
>        }
>    }
> ---CUt---
> So that, with the proposed change, the "distribution" variable will contain
> a new reference (and even possibly an instance of another type).
> Note that this setter is dangerous; it is the same problem as in issue 349:
>  https://issues.apache.org/jira/browse/MATH-349
> Hence I'd remove the setter entirely.
>
> What do you think?
>
>
> 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


Mime
View raw message