[
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
