commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Phil Steitz <>
Subject Re: [math] RandomDataImpl refactoring
Date Sun, 06 Nov 2011 16:27:44 GMT
On 11/6/11 5:44 AM, Gilles Sadowski wrote:
> On Sat, Nov 05, 2011 at 11:03:40PM -0700, Phil Steitz wrote:
>> On 11/5/11 6:38 PM, Gilles Sadowski wrote:
>>> On Sat, Nov 05, 2011 at 08:47:18AM -0700, Phil Steitz wrote:
>>>> On 11/5/11 7:04 AM, Luc Maisonobe wrote:
>>>>> Le 05/11/2011 08:29, Phil Steitz a écrit :
>>>>>> The comments in MATH-701 included a couple of suggestions for
>>>>>> refactoring RandomDataImpl.
>>>>>> 1) Eliminate the lazy initialization of the non-secure and secure
>>>>>> generators.  Have the constructor initialize the generators
>>>>>> instead.  I am fine with this for the non-secure generator, but
>>>>>> initializing a secure random generator can be quite slow, so that
>>>>>> not a good idea.
>>>>> I agree.
>>>>>> 2) Split out the secure stuff into a separate class, possibly a
>>>>>> subclass.  I am ambivalent on this one, as I see RandomDataImpl as
>>>>>> utility class and it is convenient to have the most commonly used
>>>>>> data generation utilities bundled together.  The "secure" methods
>>>>>> only generate hex strings, ints and longs.  I have never had the
>>>>>> need or heard of the need for "secure" gaussians or the other
>>>>>> non-secure deviates.  Has anyone else?  Any comments either way on
>>>>> The only needs for secure random generators I know of is for creating
>>>>> crypto keys. I feal this is out of scope of our library. For sure,
>>>>> crypto is a mathematical thing, but we don't provide anything except
>>>>> these random generation, which in fact does not do anything fancy but
>>>>> only requires an underlying secure generator.
>>>>> So I would be happy to completely remove this stuff.
>>> I agree that this seems like a utility that does not nicely fit into Commons
>>> Math.
>>>> What I have used these things for is generating session IDs and
>>>> integer object IDs that I did not want to be easily predictable.  I
>>>> would prefer to keep the methods that are there.
>>> IIUC this example, it indeed has nothing to do with a "mathematical"
>>> application; the feature thus rather belongs in a toolkit for handling
>>> collections of things (like unique IDs).
>> [math] is used by lots of different kinds of applications.  Some in
>> physical science, some in finance, some web applications.  The
>> random data generation methods are, like the optimization and other
>> features in [math], broadly useful. 
> The RNG feature is much more broadly useful (in a scientific context) than
> the secure RNG feature.
> CM is not going to provide e.g. web services boiler-plate code just because
> web services could make use of math algorithms; that's backward reasoning.
> The "...Secure..." methods exist. I don't say "Drop them. Period." Just make
> it clear that thare are not on the same footing as all the other "next..."
> methods.
>>> In "RandomDataImpl", what differentiates "secure" from "non-secure" RNGs is
>>> that for the latter, one can use one of the many _internal_ implementations
>>> of a RNG, while for the former one can only use external ones.
>> I don't get this.  It is useful to be able to generate random longs
>> and hex strings that are not easily predictable.  That is what the
>> "nextSecureXxx" methods are for.  Again, what users do with these
>> capabilities is up to them.  I personally use them in applications
>> and see no reason to drop them.
> Not everything that is useful must be provided by Commons Math.
> The above was stressing that CM provides many implementations of a RNG, but
> none of them are secure.
> The "secure" methods in "RandomDataImpl" seems to be just syntactic sugar to
> call the standard "SecureRandom" class. I think that this difference in
> status should be reflected somehow, i.e. by moving the maethods in another
> class.
>>> As I mentioned on the JIRA page, not all the "next..." functions can be
>>> "secure". This is inconsistent with respect to the "SecureRandom" class
>>> inheriting from "Random" (thus allowing to call any "next..." methods,
>>> albeit benefiting from the "security").
>> So what?  The useful ones are ints, longs and hex strings.
> I see no reason to forbid something that comes for free by inheritance!
>>> Finally, the additional layer imposed by the "security" management would
>>> seem to also point to having this functionality in another class.
>>> I'd also suggest that the "RandomData" interface be merged with its unique
>>> implementation (a.k.a. "RandomDataImpl").
>> +1 - I was planning to suggest this.
>>> If keeping the "secure" utilities, they would go in a new
>>> "SecureRandomData", which can contain only some of the "next..." as you
>>> suggest that not all of them are really needed or useful (in which case I
>>> wonder why the standard "SecureRandom" inherits from "Random"...).
>> I see no reason to pull out 3 methods into a new class.
> I gave the reasons here and in the other post.
> There is no minimum-number-of-methods rule for class creation.

Sorry, I am still -1 for this change, for the reasons stated.  The
"secure" methods have been then since 1.0 and are useful.  The
inheritance approach is overly complicated and unnecessary.  Unless
some users chime in with the view that the class will be easier to
use split out, there is no reason to change this.

> Gilles
> ---------------------------------------------------------------------
> To unsubscribe, e-mail:
> For additional commands, e-mail:

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message