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

Phil Steitz commented on MATH-349:
----------------------------------

I agree that both (1) and (2) are problems.  The private helpers would be OK to 
address (1), but I am also OK with deprecating the setters and just setting the 
fields directly (per MATH-348) in the constructor.  As stated above, I am also 
+1 on deprecating the pluggability of the normal impl, which will (eventually) 
address (2).  I guess to address (2) now we either need to modify setNormal to 
update the parameters of the normal instance as setMean does or change the 
normal approximation implementation to not depend on the parameters of the 
distribution (which addresses the problem of z being updated).  Its probably 
easiest and best in the long term to take the first approach, documenting the 
fact that z is going to be updated (both in setNormal and constructor javadoc).

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

Reply via email to