commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [math] RandomDataImpl refactoring
Date Sun, 06 Nov 2011 12:44:33 GMT
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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message