You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by "Gilles (JIRA)" <ji...@apache.org> on 2010/03/05 16:16:45 UTC
[jira] Created: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
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":
---CUT---
public PoissonDistributionImpl(double p, NormalDistribution z) {
super();
setNormal(z);
setMean(p);
}
---CUT---
(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.
[jira] Updated: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz updated MATH-349:
-----------------------------
Fix Version/s: 3.0
Leaving open until deprecated constructors are removed in 3.0.
> 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
> Fix For: 3.0
>
>
> 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.
[jira] Commented: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12842271#action_12842271 ]
Phil Steitz commented on MATH-349:
----------------------------------
The reason this constructor exists is to allow users to plug in an alternative normal distribution implementation to be used in computing normal approximations. I don't see 1) as a serious issue, but I am +1 on deprecating the setters with aim to make this class immutable in 3.0. 2) is a harder problem, as there is no requirement that a NormalDistribution be clonable. I see three solutions, none of which are particularly appealing:
1) leave as is and specify in the javadoc that z is going to be modified
2) change the implementation to avoid changing the parameters of z
3) deprecate the constructor altogether
I vote for 1) + 3) - update the javadoc, but deprecate. If we get complaints before 3.0, we can reconsider; otherwise eliminate in 3.0
> 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":
> ---CUT---
> public PoissonDistributionImpl(double p, NormalDistribution z) {
> super();
> setNormal(z);
> setMean(p);
> }
> ---CUT---
> (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.
[jira] Commented: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Resolved: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles resolved MATH-349.
-------------------------
Resolution: Fixed
Removed deprecated code. Classes are now immutable.
> 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
> Fix For: 3.0
>
>
> 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.
[jira] Issue Comment Edited: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12843047#action_12843047 ]
Phil Steitz edited comment on MATH-349 at 3/9/10 1:47 PM:
----------------------------------------------------------
Issue (2) in the bug description remains open. I think we should leave this issue open until we have a solution for this. As stated above, I can see two ways to fix this:
a) modify the normal approximation implementation so that it does not change the parameters of z
b) eliminate pluggability of z (i.e., deprecate and then remove the constructor that accepts a normal instance as a parameter)
a) could be accomplished in 2.x, but it would complicate the code and could be bad for numerics. My vote is for b) - add a warning (about safety as well as deprecation), deprecate now and if we get no complaints before 3.0, remove then. Leave the issue open with fix version 3.0.
was (Author: psteitz):
Issue (2) in the bug description remains open. I think we should leave this issue open until we have a solution for this. As stated above, I can see two ways to fix this:
a) modify the normal approximation implementation so that it does not change the parameters of z
b) eliminate pluggability of z (i.e., deprecate and then remove the constructor that accepts a normal instance as a parameter)
a) could be accomplished in 2.x, but it would complicate the code and could be bad for numerics. My vote is for b) - add a warning (about safety as well as deprecation), deprecate now and if we get no complaints before 3.0, remove then.
> 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.
[jira] Commented: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12915051#action_12915051 ]
Gilles commented on MATH-349:
-----------------------------
{{ChiSquaredDistributionImpl}} made immutable in revision 1001533.
> 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
> Fix For: 3.0
>
>
> 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.
[jira] Reopened: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Phil Steitz reopened MATH-349:
------------------------------
Issue (2) in the bug description remains open. I think we should leave this issue open until we have a solution for this. As stated above, I can see two ways to fix this:
a) modify the normal approximation implementation so that it does not change the parameters of z
b) eliminate pluggability of z (i.e., deprecate and then remove the constructor that accepts a normal instance as a parameter)
a) could be accomplished in 2.x, but it would complicate the code and could be bad for numerics. My vote is for b) - add a warning (about safety as well as deprecation), deprecate now and if we get no complaints before 3.0, remove then.
> 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.
[jira] Commented: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ 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.
[jira] Commented: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12914889#action_12914889 ]
Gilles commented on MATH-349:
-----------------------------
{{PoissonDistributionImpl}} made immutable in revision 1001331.
> 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
> Fix For: 3.0
>
>
> 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.
[jira] Commented: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Phil Steitz (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12843146#action_12843146 ]
Phil Steitz commented on MATH-349:
----------------------------------
Sorry, I missed the constructor deprecations in r920852
> 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.
[jira] Updated: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-349:
------------------------
Description:
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?
was:
In the following excerpt from class "PoissonDistributionImpl":
---CUT---
public PoissonDistributionImpl(double p, NormalDistribution z) {
super();
setNormal(z);
setMean(p);
}
---CUT---
(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?
> 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.
[jira] Commented: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12843052#action_12843052 ]
Gilles commented on MATH-349:
-----------------------------
The same kind of problem also exists in "ChiSquaredDistributionImpl.java".
In both classes, the constructor that takes the distribution as a parameter has been deprecated.
> 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.
[jira] Resolved: (MATH-349) Dangerous code in
"PoissonDistributionImpl"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-349?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles resolved MATH-349.
-------------------------
Resolution: Fixed
Issue is partially fixed (removed the consistency problem) in revision 920852.
Setters were marked as deprecated.
> 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.