[MATH-734] Code cleanup: "ISAACRandom"

• Type: Improvement
• Status: Resolved
• Priority: Minor
• Resolution: Won't Fix
• Fix Version/s: 4.0
• Assignee: Unassigned
• Reporter: Gilles Sadowski
• Created: 18/Jan/12 14:46
• Updated: 25/Feb/16 12:19
• Resolved: 25/Feb/16 12:19

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)

  if (seed == null) {
    setSeed(System.currentTimeMillis() + System.identityHashCode(this));
    return;
  }
  

  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.

Issue Links

• contains: MATH-1309 RNG: public "setSeed" method should not be called from inside the constructor
• is superceded by: MATH-1319 Refactor the package "o.a.c.m.random"