You should indicate in the javadoc of the class that it's thread-safe.
You should use a private lock object instead of
synchronized
methods:private final Object lock = new Object(); ... public synchronized List<Integer> getPrimesUptoN(int n) { synchronized (lock) { validatePositive(n); validateOutOfBound(n); ... return Collections.unmodifiableList(computePrimesUptoN(n)); } }
Consider the following code:
final PrimeUtil primeUtil = new PrimeUtil();
synchronized (primeUtil) {
}
Anyone can lock on this instance which could lead to poor performance or deadlock.
See also: Effective Java 2nd Edition, Item 70: Document thread safety
1.
private static final int DEFAULT_SIZE = 100;
I think DEFAULT_LOOK_AHEAD
would be more descriptive. DEFAULT_SIZE
is a little bit misleading.
- I'd rename the field to
primeCache
and remove the comment:
// cache of primes. public final List<Integer> primes;
See Clean Code by Robert C. Martin, Don’t Use a Comment When You Can Use a Function or a Variable, p67
1.
primes = new ArrayList<Integer>();
You could save a line by putting this on the same line as the declaration:
public final List<Integer> primes = new ArrayList<Integer>();
I think it would be a little bit easier to follow.
I'd put the validate methods at the end of the file. The are not so important to be the first methods.
if (primeIndex != -1 && primeIndex != primes.size()) {
I'd create a NOT_FOUNDNOT_FOUND
constant for -1
here for better readability.