On 11/6/11 10:20 AM, Gilles Sadowski wrote:
> On Sun, Nov 06, 2011 at 09:27:44AM -0700, Phil Steitz wrote:
>> 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 is
>>>>>>>> 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 a
>>>>>>>> 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
>>>>>>>> this?
>>>>>>> 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.
> Overly complicated? Certainly not:
> ---
> SecureRandomData s = new SecureRandomData();
> ---
> Unless you count the additional typing of the "Secure" string as
> "complicated"!
>
> Polymorphism will make some things even simpler; and you can then add more
> of the specific "secure" things (like "Provider"), all of those
> unnecessarily pollute a "RandomData" class that doesn't need to be aware of
> cryptographic constraints.
>
>> 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.
> 1. Luc "was happy removing all the stuff".
> 2. Sebb was inclined to make the field final.
> 3. I agree with both.
> That's three to one, if I count correctly.
>
> I don't have a big problem with keeping the functionality, if you insist on
> that point.
> I just suggested that it should stand out for what it is: syntactic sugar.
Look at the code for nextSecureHex. That is not just "sugar." Your
opinion of what is "sugar" is just that - your opinion. I would
prefer not to have to rewrite that code in my applications. It is
not "sugar" to me or anyone else who is using it. There are lots of
funny little functions in what used to be MathUtils and the whole
raft of Function stuff that I did not complain about because I
assume you or others found them useful. I don't think it is
valuable or productive to start arguing about this now. Please just
accept the fact that this useful functionality has been there since
1.0 and it is not going to be dropped.
> IMHO, this alone makes the thing easier to use because you don't have to
> have such a long explanation as currently exists in "RandomDataImpl" to make
> the distinction between secure and non-secure.
> With the refactoring, you can just state: "Application that require secure
> RNG should use the {@link SecureRandomData} class". And in that class, you
> give example of situations where the stuff could be useful (like session
> IDs) without polluting a class like "RandomData".
> I must add that you are yourself making a point that the utilities are
> different, when stating that "secure" RNGs are "slow" to initialize.
> And there are other properties (non-deterministic) that might make them
> unsuitable for scientific usage.
> Separation would go a good deal towards self-documenting code, thus _better_
> documented code. [Say, people who wouldn't know CM, would readily spot a
> class named "SecureRandomData". Being in line with the design of the Java
> standard hierarchy for this stuff makes it also easier.]
I would like to hear from some actual users before changing this.
Phil
>
>
> Gilles
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
