commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <j...@apache.org>
Subject [jira] Commented: (MATH-349) Dangerous code in "PoissonDistributionImpl"
Date Mon, 08 Mar 2010 16:49:30 GMT

    [ 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.


Mime
View raw message