On May 3, 2014, at 6:55 PM, Joe Darcy <joe.da...@oracle.com> wrote: > Hi Brian, > > I think the parsePrimes method would be better with a different name since no > parsing is occurring anymore. >
Yes, and there is no need for the try block. (The use of BitSet.stream() also reminds me we can implement a spliterator with size estimates based on the words in use, currently the stream() is created from an iterator.) > I think someone in this test the fact that Integer.MAX_VALUE is a prime > should be mentioned in taken advantage of :-) How about adding another test > method which tests some Mersenne primes for primality? [1] > > I'd prefer to see some comments on the primes method briefly explaining it > methodology. Would the running time be unacceptable (or memory usage too > large) if the limit were set to Integer.MAX_VALUE? > +1 to both of those, although perhaps the former could be done as a second iteration to get this code in? Also, probably better to create the "NavigableSet<BigInteger> primes" just once in "main", rather than creating it twice for both tests. Still skeptical about the use of a PRNG in the tests, see my comments in a previous email: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-April/026579.html Paul.
