Looks good.

Random::

- Seems fine.


ThreadLocalRandom::

- I don't understand the point of having a writeObject() if the readResolve() 
ignores the result. My expectation for a serialized TLR might be that upon 
de-serialization the seeding state is restored. If that isn't provided, why 
offer any serialization at all? Alternately we should be more explicit that the 
seeding state is not part of the serialization.

- There's no test for the serialization behaviour.

SplittableRandomTest::

- executeAndCatch -> assertThrows perhaps? There are a few implementations of 
assertThrows around in other tests (which haven't been collected into a library 
yet).


Mike

On Aug 27 2013, at 12:06 , Paul Sandoz wrote:

> Updated:
> 
>  http://cr.openjdk.java.net/~psandoz/tl/JDK-8023155-Random-TLR-SR-sync/webrev/
> 
> - we reverted the addition of the new next* methods on Random. The 
> stream-based methods remain unchanged. Decided to be extra conservative, 
> since there may be sub-classes that define such methods (not unusual e.g. 
> just like TLR) and the contract might be different to what we specify 
> (probably most likely around error handling but there could be other subtle 
> issues).
> 
> - the stream-based int origin/bound support is defined using nextInt methods 
> rather than next, which should be better when used with existing sub-classes.
> 
> - some additional tests were added for double-related origin/bounds methods 
> of Random, ThreadLocalRandom, and SplittableRandom.
> 
> Note for TLR default constructors the seed is not yet based on the same 
> algorithm as SplittableRandom. As discussed in a previous email we cannot do 
> that until the hash seed functionality is removed from WeakHashMap and 
> Hashtable.

Soon I hope.



> Paul.
> 
> 

Reply via email to