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. Gilles --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
