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 (Created) (JIRA)" <ji...@apache.org> on 2012/01/18 15:48:39 UTC
[jira] [Created] (MATH-734) Code cleanup: "ISAACRandom"
Code cleanup: "ISAACRandom"
---------------------------
Key: MATH-734
URL: https://issues.apache.org/jira/browse/MATH-734
Project: Commons Math
Issue Type: Improvement
Reporter: Gilles
Assignee: Gilles
Priority: Minor
Fix For: 3.0
In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
Other points I'd want to handle:
* Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
* (Related to the above) I'd remove the "transient" keyword.
* The contents of method "allocArrays" should be moved to within the constructor.
* It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
* All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-734) Code cleanup: "ISAACRandom"
Posted by "Gilles (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-734?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-734:
------------------------
Description:
In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
Other points I'd want to handle:
* Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
* (Related to the above) I'd remove the "transient" keyword.
* The contents of method "allocArrays" should be moved to within the constructor.
* It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
* All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
* This code (line 130)
{code}
if (seed == null) {
setSeed(System.currentTimeMillis() + System.identityHashCode(this));
return;
}
{code}
should probably be removed: it is safer to consider passing a null reference as a user error.
was:
In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
Other points I'd want to handle:
* Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
* (Related to the above) I'd remove the "transient" keyword.
* The contents of method "allocArrays" should be moved to within the constructor.
* It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
* All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
> Code cleanup: "ISAACRandom"
> ---------------------------
>
> Key: MATH-734
> URL: https://issues.apache.org/jira/browse/MATH-734
> Project: Commons Math
> Issue Type: Improvement
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.0
>
>
> In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
> Other points I'd want to handle:
> * Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
> * (Related to the above) I'd remove the "transient" keyword.
> * The contents of method "allocArrays" should be moved to within the constructor.
> * It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
> * All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
> * This code (line 130)
> {code}
> if (seed == null) {
> setSeed(System.currentTimeMillis() + System.identityHashCode(this));
> return;
> }
> {code}
> should probably be removed: it is safer to consider passing a null reference as a user error.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-734) Code cleanup: "ISAACRandom"
Posted by "Gilles (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-734?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-734:
------------------------
Fix Version/s: (was: 3.0)
3.1
Postponing. Some of the same (non-blocking) issues apply to other RNG implementations; this can be discussed later.
> Code cleanup: "ISAACRandom"
> ---------------------------
>
> Key: MATH-734
> URL: https://issues.apache.org/jira/browse/MATH-734
> Project: Commons Math
> Issue Type: Improvement
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.1
>
>
> In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
> Other points I'd want to handle:
> * Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
> * (Related to the above) I'd remove the "transient" keyword.
> * The contents of method "allocArrays" should be moved to within the constructor.
> * It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully initialized object.
> * All initializations should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
> * This code (line 130)
> {code}
> if (seed == null) {
> setSeed(System.currentTimeMillis() + System.identityHashCode(this));
> return;
> }
> {code}
> should probably be removed: it is safer to consider passing a null reference as a user error.
> * I'd remove the constructor taking a {{long}} argument. It is not obvious how the code will use the argument. I'd rather offer that alternative, in the class documentation, as an example of how to initialize the "array of integers" argument.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-734) Code cleanup: "ISAACRandom"
Posted by "Gilles (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-734?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-734:
------------------------
Description:
In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
Other points I'd want to handle:
* Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
* (Related to the above) I'd remove the "transient" keyword.
* The contents of method "allocArrays" should be moved to within the constructor.
* It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully initialized object.
* All initializations should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
* This code (line 130)
{code}
if (seed == null) {
setSeed(System.currentTimeMillis() + System.identityHashCode(this));
return;
}
{code}
should probably be removed: it is safer to consider passing a null reference as a user error.
* I'd remove the constructor taking a {{long}} argument. It is not obvious how the code will use the argument. I'd rather offer that alternative, in the class documentation, as an example of how to initialize the "array of integers" argument.
was:
In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
Other points I'd want to handle:
* Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
* (Related to the above) I'd remove the "transient" keyword.
* The contents of method "allocArrays" should be moved to within the constructor.
* It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
* All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
* This code (line 130)
{code}
if (seed == null) {
setSeed(System.currentTimeMillis() + System.identityHashCode(this));
return;
}
{code}
should probably be removed: it is safer to consider passing a null reference as a user error.
* I'd remove the constructor taking a {{long}} argument. It is not obvious how the code will use the argument. I'd rather offer that alternative, in the class documentation, as an example of how to initialize the "array of integers" argument.
> Code cleanup: "ISAACRandom"
> ---------------------------
>
> Key: MATH-734
> URL: https://issues.apache.org/jira/browse/MATH-734
> Project: Commons Math
> Issue Type: Improvement
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.0
>
>
> In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
> Other points I'd want to handle:
> * Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
> * (Related to the above) I'd remove the "transient" keyword.
> * The contents of method "allocArrays" should be moved to within the constructor.
> * It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully initialized object.
> * All initializations should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
> * This code (line 130)
> {code}
> if (seed == null) {
> setSeed(System.currentTimeMillis() + System.identityHashCode(this));
> return;
> }
> {code}
> should probably be removed: it is safer to consider passing a null reference as a user error.
> * I'd remove the constructor taking a {{long}} argument. It is not obvious how the code will use the argument. I'd rather offer that alternative, in the class documentation, as an example of how to initialize the "array of integers" argument.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-734) Code cleanup: "ISAACRandom"
Posted by "Gilles (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-734?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-734:
------------------------
Fix Version/s: (was: 3.1)
> Code cleanup: "ISAACRandom"
> ---------------------------
>
> Key: MATH-734
> URL: https://issues.apache.org/jira/browse/MATH-734
> Project: Commons Math
> Issue Type: Improvement
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
>
> In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
> Other points I'd want to handle:
> * Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
> * (Related to the above) I'd remove the "transient" keyword.
> * The contents of method "allocArrays" should be moved to within the constructor.
> * It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully initialized object.
> * All initializations should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
> * This code (line 130)
> {code}
> if (seed == null) {
> setSeed(System.currentTimeMillis() + System.identityHashCode(this));
> return;
> }
> {code}
> should probably be removed: it is safer to consider passing a null reference as a user error.
> * I'd remove the constructor taking a {{long}} argument. It is not obvious how the code will use the argument. I'd rather offer that alternative, in the class documentation, as an example of how to initialize the "array of integers" argument.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (MATH-734) Code cleanup: "ISAACRandom"
Posted by "Gilles (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/MATH-734?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Gilles updated MATH-734:
------------------------
Description:
In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
Other points I'd want to handle:
* Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
* (Related to the above) I'd remove the "transient" keyword.
* The contents of method "allocArrays" should be moved to within the constructor.
* It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
* All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
* This code (line 130)
{code}
if (seed == null) {
setSeed(System.currentTimeMillis() + System.identityHashCode(this));
return;
}
{code}
should probably be removed: it is safer to consider passing a null reference as a user error.
* I'd remove the constructor taking a {{long}} argument. It is not obvious how the code will use the argument. I'd rather offer that alternative, in the class documentation, as an example of how to initialize the "array of integers" argument.
was:
In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
Other points I'd want to handle:
* Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
* (Related to the above) I'd remove the "transient" keyword.
* The contents of method "allocArrays" should be moved to within the constructor.
* It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
* All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
* This code (line 130)
{code}
if (seed == null) {
setSeed(System.currentTimeMillis() + System.identityHashCode(this));
return;
}
{code}
should probably be removed: it is safer to consider passing a null reference as a user error.
> Code cleanup: "ISAACRandom"
> ---------------------------
>
> Key: MATH-734
> URL: https://issues.apache.org/jira/browse/MATH-734
> Project: Commons Math
> Issue Type: Improvement
> Reporter: Gilles
> Assignee: Gilles
> Priority: Minor
> Fix For: 3.0
>
>
> In revision 1232899, I started to clean up the code (mainly, removing the one-letter instance variables, that can easily be confused with local ones within methods, making the code harder to understand and maintain).
> Other points I'd want to handle:
> * Should "Serializable" be implemented for such classes? I think not; especially if it supposed to be used for "secure" applications.
> * (Related to the above) I'd remove the "transient" keyword.
> * The contents of method "allocArrays" should be moved to within the constructor.
> * It is not recommended to call non-final "public" methods ("setSeed") from within the constructor, because an overriding code could access a not fully uninitialized object.
> * All initialization should take place within a single, most general, constructor and the other constructors should call that one (using the {{this(...)}} statement).
> * This code (line 130)
> {code}
> if (seed == null) {
> setSeed(System.currentTimeMillis() + System.identityHashCode(this));
> return;
> }
> {code}
> should probably be removed: it is safer to consider passing a null reference as a user error.
> * I'd remove the constructor taking a {{long}} argument. It is not obvious how the code will use the argument. I'd rather offer that alternative, in the class documentation, as an example of how to initialize the "array of integers" argument.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira