Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Message-ID: <1124763886.138341268066970692.JavaMail.jira@brutus.apache.org>
Date: Mon, 8 Mar 2010 16:49:30 +0000 (UTC)
From: "Gilles (JIRA)" <jira@apache.org>
To: issues@commons.apache.org
Subject: [jira] Commented: (MATH-349) Dangerous code in
 "PoissonDistributionImpl"
In-Reply-To: <1370468705.99551267802205840.JavaMail.jira@brutus.apache.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


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

Gilles commented on MATH-349:
-----------------------------

Problem (1)
In principle, it is not safe to call overridable methods in the constructor, so, if only to promote coding quality, can I implement private ("helper") setter methods that shall be called from within the constructors and from the public setters?

Problem (2)
In "PoissonDistributionImpl.java", there is this code:

{code:title=PoissonDistributionImpl.java|borderStyle=solid}

public PoissonDistributionImpl(double p, NormalDistribution z) {
    super();
    setNormal(z);
    setMean(p);
}

public void setMean(double p) {
    // ...
    this.mean = p;
    normal.setMean(p);
    normal.setStandardDeviation(Math.sqrt(p));
}

public void setNormal(NormalDistribution value) {
    normal = value;
}
{code}

In the constructor, the code makes sure that the "mean" of this class and the mean of the "normal" object are consistent.
But there is no such guarantee anymore when calling the "setNormal" method. [The comment warns the user that he is responsible for setting the right parameter in "value" but this is far from fool-proof...]


> Dangerous code in "PoissonDistributionImpl"
> -------------------------------------------
>
>                 Key: MATH-349
>                 URL: https://issues.apache.org/jira/browse/MATH-349
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Gilles
>            Priority: Minor
>
> In the following excerpt from class "PoissonDistributionImpl":
> {code:title=PoissonDistributionImpl.java|borderStyle=solid}
>     public PoissonDistributionImpl(double p, NormalDistribution z) {
>         super();
>         setNormal(z);
>         setMean(p);
>     }
> {code}
> (1) Overridable methods are called within the constructor.
> (2) The reference "z" is stored and modified within the class.
> I've encountered problem (1) in several classes while working on issue 348. In those cases, in order to remove potential problems, I copied/pasted the body of the "setter" methods inside the constructor but I think that a more elegant solution would be to remove the "setters" altogether (i.e. make the classes immutable).
> Problem (2) can also create unexpected behaviour. Is it really necessary to pass the "NormalDistribution" object; can't it be always created within the class?

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.