Skip to main content
probably improves list formattingű
Source Link
palacsint
  • 29.9k
  • 9
  • 81
  • 156
  1. containsPrime could be containsNthPrime (for consistency with getNthPrime).

  2. For this:

      if (to <= 1) {
          throw new IllegalArgumentException("Illegal range to " + to);
      }
      if (to > MAX_PRIME_ALLOWED) {
          throw new IllegalArgumentException("Memory-limited to the largest prime " + MAX_PRIME_ALLOWED
                  + " (which is the " + MAX_PRIME_NTH + "th prime)");
      }
    
  1. In System.out.printf use %n instead of \n. The former outputs the correct platform-specific line separator.

  2. It's not unambiguous who is this comment for:

       * NEVER Violate the prime directive!
    

It's not unambiguous who is this comment for. The client or the developer of PrimeReserveInt? It's on a private field but sounds like a warning to clients of the class. As a client, what should I do?

  1. containsPrime could be containsNthPrime (for consistency with getNthPrime).

  2.   if (to <= 1) {
          throw new IllegalArgumentException("Illegal range to " + to);
      }
      if (to > MAX_PRIME_ALLOWED) {
          throw new IllegalArgumentException("Memory-limited to the largest prime " + MAX_PRIME_ALLOWED
                  + " (which is the " + MAX_PRIME_NTH + "th prime)");
      }
    
  1. In System.out.printf use %n instead of \n. The former outputs the correct platform-specific line separator.

  2.    * NEVER Violate the prime directive!
    

It's not unambiguous who is this comment for. The client or the developer of PrimeReserveInt? It's on a private field but sounds like a warning to clients of the class. As a client, what should I do?

  1. containsPrime could be containsNthPrime (for consistency with getNthPrime).

  2. For this:

     if (to <= 1) {
         throw new IllegalArgumentException("Illegal range to " + to);
     }
     if (to > MAX_PRIME_ALLOWED) {
         throw new IllegalArgumentException("Memory-limited to the largest prime " + MAX_PRIME_ALLOWED
                 + " (which is the " + MAX_PRIME_NTH + "th prime)");
     }
    
  1. In System.out.printf use %n instead of \n. The former outputs the correct platform-specific line separator.

  2. It's not unambiguous who is this comment for:

      * NEVER Violate the prime directive!
    

The client or the developer of PrimeReserveInt? It's on a private field but sounds like a warning to clients of the class. As a client, what should I do?

typo fix
Source Link
palacsint
  • 29.9k
  • 9
  • 81
  • 156

(It could also save you a few string unnecessary string concatenation.)

(It could also save you a few string unnecessary string concatenation.)

(It could also save you a few unnecessary string concatenation.)

Source Link
palacsint
  • 29.9k
  • 9
  • 81
  • 156

A few generic notes:

  1. PrimeReserveInt: I think it could be smaller with separated responsibilities. I would put the prime generation (and related) logic to a separate class and put the (prime)state reference and thread handling to another one. (What would you extract out if you wanted to change prime generation to another algorithm?)

  2. I don't see any reason to use AtomicBoolean instead of ReentrantLock for locking. It supports tryLock. (If there is a reason you should document it somehow.)

  3. You could use a CountDownLatch instead of the wait/notify.

     private final CountDownLatch replaced = new CountDownLatch(1);
    
     synchronized void waitReplaced() {
         try {
             replaced.await();
         } catch (InterruptedException e) {
             Thread.currentThread().interrupt();
         }
     }
    
     synchronized void replaced() {
         replaced.countDown();
     }
    

See: Effective Java, 2nd edition, Item 69: Prefer concurrency utilities to wait and notify

  1. containsPrime could be containsNthPrime (for consistency with getNthPrime).

  2.   if (to <= 1) {
          throw new IllegalArgumentException("Illegal range to " + to);
      }
      if (to > MAX_PRIME_ALLOWED) {
          throw new IllegalArgumentException("Memory-limited to the largest prime " + MAX_PRIME_ALLOWED
                  + " (which is the " + MAX_PRIME_NTH + "th prime)");
      }
    

Validation would be readable with Guava's Preconditions:

    checkArgument(to > 1, "Illegal range to %s", to);
    checkArgument(to <= MAX_PRIME_ALLOWED, 
        "Memory-limited to the largest prime %s (which is the %sth prime)",
            MAX_PRIME_ALLOWED, MAX_PRIME_NTH);

(It could also save you a few string unnecessary string concatenation.)

  1. In System.out.printf use %n instead of \n. The former outputs the correct platform-specific line separator.

  2.    * NEVER Violate the prime directive!
    

It's not unambiguous who is this comment for. The client or the developer of PrimeReserveInt? It's on a private field but sounds like a warning to clients of the class. As a client, what should I do?

  1. This comment should be on the class declaration instead of the constructor:

     /**
      * ...
      * This class is fully thread-safe.
      * ...
      */
     public PrimeReserveInt() {
         ...
     }