Hello.
>>>> [...]
> >>>
> >>>>
> >>>> 2. Add a method BitSet<T> getSupport() to the RandomSource enum, where T
> >>>> is an enum that can be expanded as more features are added. Initially is
> >>>> would be:
> >>>
> >>> +1
> >>> With "EnumSet", I guess.
> >>
> >> Yes.
> >>
> >>>
> >>>>
> >>>> enum RandomSourceSupport {
> >>>> /** The source supported the {@link JumpableUniformRandomProvider}
> >>>> interface. */
> >>>> JUMPABLE,
> >>>> /** The source supported the {@link
> >>>> LongJumpableUniformRandomProvider}
> >>>> interface. */
> >>>> LONG_JUMPABLE,
> >>>> }
> >>>
> >>> +1
> >>>
> >>>>
> >>>> Currently the RandomSource only has a isNativeSeed(Object) method. So
> >>>> adding methods does add API clutter. It would be possible to also add
> >>>> getNativeSeedLength() method as this information (available in the
> >>>> javadoc) is now available in the factory code that builds generators.
> >>>
> >>> +1
> >>
> >>
> >> So this would add to RandomSource:
> >>
> >> int getSeedByteLength()
> >> EnumSet<RandomSourceSupport> getSupport()
> >
> > I think that the API should not specify the type of "Set".
> > Also, the enum should be an inner class. Thus:
> >
> > public class RandomSource {
> > // ...
> >
> > public enum Support {
> > JUMPABLE,
> > // etc.
> > }
> >
> > public Set<Support> getSupport() {
> > final Set<Support> s = new EnumSet<>(Support.class);
> >
> > // ...
> >
> > return s;
> > }
> > }
>
> On reflection I think the use of Set<Enum> is not the best choice. Currently
> we are only wanting to support the question of if this is possible:
>
> (JumpableUniformRandomProvider) RandomSource.create(…)
> (LongJumpableUniformRandomProvider) RandomSource.create(…)
>
> The use of an enum is not a good fit here as any LongJumpable is also a
> Jumpable due to the inheritance hierarchy. So the set of an enum that
> specifies one or the other or both can be incorrectly used to be LongJumpable
> but not Jumpable.
I'm not sure I follow: "incorrectly used" by who?
The library will return a consistent set (if it contains "LONG_JUMPABLE",
it will also contain "JUMPABLE").
>
> Plus it would require returning a copy of the set for each invocation to get
> the supported functionality.
Is that a problem?
[Assuming this is setup/one time initialization of an RNG.]
> And either storing the set per enum value or computing is dynamically which
> is possible...
Yes; an implementation detail.
>
> So perhaps a method that is the inverse of the Class::isAssignableFrom
> method. Since the RandomSourceInternal knows the RNG class this can be
> implemented as:
>
> /**
> * Determines if the class represented by this random source is either the
> same
> * as, or is a subclass or subinterface of, the class or interface represented
> * by the specified {@code Class} parameter. It returns true if so; otherwise
> it returns
> * false.
> *
> * @param type the {@code Class} object to be checked
> * @return the boolean value indicating whether the class of this random
> source
> * can be assigned to objects of the specified type
> */
> public boolean isAssignableTo(Class<?> type) {
> return type.isAssignableFrom(internalIdentifier.getRng());
> }
>
> Allowing:
>
> RandomSource source = ...
> If (source.isAssignableTo(JumpableUniformRandomProvider.class) {
> (JumpableUniformRandomProvider) RandomSource.create(source);
> } else {
> // fail ...
> }
Then, what have you gained over "instanceof"?
Yes, it will avoid an instantiation; but the price seems to be an ugly API
(IMHO).
>
> This option is actually the easiest to implement and if more interfaces are
> added later then nothing needs to be done since it is self-detecting.
Yes, but then I'd keep that for the internal implementation of "support":
public enum RandomSource {
// ...
public boolean isLongJumpable() {
return isAssignableTo(LongJumpableUniformRandomProvider.class);
}
private boolean isAssignableTo(Class<?> type) {
return type.isAssignableFrom(internalIdentifier.getRng());
}
}
Do you foresee many other "properties" that would warrant sparing
a trivial method for each?
Gilles
> >
> >>
> >> The first should return the native seed size in bytes. Currently the
> >> javadoc
> >> contains the native seed length. This is the length of the seed array or 1
> >> if the seed is a Long/Integer.
> >>
> >> I think having the seed size in bytes is more generic and it doesn’t matter
> >> if the seed is an array or not. So perhaps the javadoc should contain the
> >> human readable array length (if relevant) but the code uses the generic
> >> byte
> >> length allowing:
> >
> > +1
> >
> >>
> >> SecureRandom sr = new SecureRandom();
> >> RandomSource source = RandomSource.MWC_256;
> >> UniformRandomProvider rng = RandomSource.create(source,
> >> sr.generateSeed(source.getSeedByteLength());
> >>
> >> In the case where the seed is a Long/Integer I suggest dropping the
> >> javadoc:
> >> "Native seed size: 1” as it is redundant.
> >
> > +1
> >
> > Regards,
> > Gilles
> >
> >>>
> >>>>
> >>>> Any other suggestions?
> >>>>
> >>>> Alex
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
