[
https://issues.apache.org/jira/browse/RNG-143?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17358886#comment-17358886
]
Alex Herbert commented on RNG-143:
----------------------------------
Firstly I just put the two new instance methods into RandomSource and ran:
{noformat}
mvn clean test package install site site:stage
{noformat}
This works fine with the source code as is. However when doing the same with
the examples profile enabled there is a compilation error. This is no longer
valid:
{code:java}
RandomSource randomSource = ...;
RandomSource.create(randomSource, null)
{code}
This is intended to call the factory method with a null seed. It now thinks it
should be calling the instance method with the randomSource as an Object seed.
It can be fixed with a cast:
{code:java}
// Compiler want to call the instance method, effectively like this:
// RandomSource instance = RandomSource.MT;
// instance.create(Object, Object... args)
// It is a compile failure as you cannot make a static reference to a
non-static method
// Let the compiler know to call:
// RandomSource.create(RandomSource, Object, Object... args)
RandomSource.create(randomSource, (Object) null);
// Or
Object seed = null;
RandomSource.create(randomSource, seed);
{code}
I do not think this is an issue. This is a benchmark method deliberately
passing a null seed. In practice you would not pass a null seed and omit the
argument to call:
{code:java}
RandomSource.create(randomSource);
{code}
You then get a deprecation warning for using the deprecated factory method.
So adding the methods will be non-destructive for any source to compile against
the updated library, with the noted exception above.
I then updated the entire codebase to use the new instance create method. I
found some issues.
RandomSource was documented to throw an IllegalStateException when the
arguments passed to the create method were incorrect. This was not actually
tested because it actually throws an IllegalArgumentException. I updated the
documentation and added tests.
There was a test commented out in ThreadLocalRandomTest that expected an
IllegalArgumentException when used with TWO_CMRES_SELECT. The history shows it
has always been commented out. I cannot remember the reason; the test has now
been reinstated.
Code expressing a particular RandomSource can be converted with the following
regex:
{noformat}
s/RandomSource.create\(RandomSource.([A-Z0-9_]+)(, )?/RandomSource.$1.create(/
{noformat}
Code specifying the RandomSource as a variable would need a different regex.
There were not many instances of this and I did not use a regex but changed the
code manually.
There are some cases where the code will then compile but break when executed.
Here is an example that breaks:
{code:java}
RandomSource.create(RandomSource.TWO_CMRES_SELECT, null, 5, 3));
{code}
Becomes:
{code:java}
RandomSource.TWO_CMRES_SELECT.create(null, 5, 3));
{code}
Here {{null}} is intended to be the seed. But the compiler will link this to
the old static factory create method using {{null}} as the random source,
effectively calling:
{code:java}
RandomSource rs = null;
RandomSource.create(rs, 5, 3));
{code}
It should be calling the new instance create method with a null object. You can
make it do this by casting the null to object:
{code:java}
RandomSource.TWO_CMRES_SELECT.create((Object) null, 5, 3));
{code}
I made changes to all the examples and the site documentation. The old factory
method has effectively been removed.
Performing:
{noformat}
git grep 'RandomSource.create([^\)]'
{noformat}
from the top directory shows what is left. There are some documentation
examples in RandomSource and tests for the factory method.
The changes build using:
{noformat}
mvn clean test package install site site:stage -Pcommons-rng-examples
{noformat}
The PR has lots of file changes. I'll review them another day. Please have a
look and note any issues.
> Instance create() method for RandomSource
> -----------------------------------------
>
> Key: RNG-143
> URL: https://issues.apache.org/jira/browse/RNG-143
> Project: Commons RNG
> Issue Type: New Feature
> Components: simple
> Affects Versions: 1.3
> Reporter: Alex Herbert
> Priority: Trivial
> Time Spent: 10m
> Remaining Estimate: 0h
>
> This is a wish for an instance {{create}} method for RandomSource:
> {code:java}
> public RestorableUniformRandomProvider create() {
> return create(this);
> }
> public RestorableUniformRandomProvider create(Object seed,
> Object... data) {
> return create(this, seed, data);
> }
> {code}
> It is syntactic sugar to avoid typing the use of the static factory method
> and repeating {{RandomSource}} in close succession:
> {code:java}
> RandomSource.create(RandomSource.MT);
> RandomSource.create(RandomSource.MT, 12345L);
> {code}
> Becomes:
> {code:java}
> RandomSource.MT.create();
> RandomSource.MT.create(12345L);
> {code}
> It would make the use of the static factory method redundant, which could be
> deprecated.
> Opinions on this?
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
