From 483c61a82af39ba7fc9c226bf112dc8db511e429 Mon Sep 17 00:00:00 2001
From: Narendra Pathai <narendra.pathai@gmail.com>
Date: Mon, 29 Aug 2016 00:16:36 +0530
Subject: [PATCH] Some refactoring, added javadocs

---
 .../main/java/com/iluwatar/promise/App.java   | 141 +++++++++++-------
 .../java/com/iluwatar/promise/Promise.java    |  36 ++++-
 .../java/com/iluwatar/promise/Utility.java    |  46 +++---
 .../com/iluwatar/promise/PromiseTest.java     |  46 ++++--
 4 files changed, 175 insertions(+), 94 deletions(-)

diff --git a/promise/src/main/java/com/iluwatar/promise/App.java b/promise/src/main/java/com/iluwatar/promise/App.java
index 1315f0927..2b2ae78b4 100644
--- a/promise/src/main/java/com/iluwatar/promise/App.java
+++ b/promise/src/main/java/com/iluwatar/promise/App.java
@@ -29,35 +29,45 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
 /**
- * 
- * <p>The Promise object is used for asynchronous computations. A Promise represents an operation that
- * hasn't completed yet, but is expected in the future.
- * 
- * <p>A Promise represents a proxy for a value not necessarily known when the promise is created. It 
- * allows you to associate dependent promises to an asynchronous action's eventual success value or 
- * failure reason. This lets asynchronous methods return values like synchronous methods: instead of the final
- * value, the asynchronous method returns a promise of having a value at some point in the future.
- * 
+ *
+ *  The Promise object is used for asynchronous computations. A Promise represents an operation
+ *  that hasn't completed yet, but is expected in the future.
+ *
+ * <p>A Promise represents a proxy for a value not necessarily known when the promise is created. It
+ * allows you to associate dependent promises to an asynchronous action's eventual success value or
+ * failure reason. This lets asynchronous methods return values like synchronous methods: instead 
+ * of the final value, the asynchronous method returns a promise of having a value at some point 
+ * in the future.
+ *
  * <p>Promises provide a few advantages over callback objects:
  * <ul>
  * <li> Functional composition and error handling
  * <li> Prevents callback hell and provides callback aggregation
  * </ul>
- * 
+ *
  * <p>
+ * In this application the usage of promise is demonstrated with two examples:
+ * <ul>
+ * <li>Count Lines: In this example a file is downloaded and its line count is calculated.
+ * The calculated line count is then consumed and printed on console.
+ * <li>Lowest Character Frequency: In this example a file is downloaded and its lowest frequency
+ * character is found and printed on console. This happens via a chain of promises, we start with
+ * a file download promise, then a promise of character frequency, then a promise of lowest frequency
+ * character which is finally consumed and result is printed on console.
+ * </ul>
  * 
  * @see CompletableFuture
  */
 public class App {
 
-  private static final String URL = "https://raw.githubusercontent.com/iluwatar/java-design-patterns/Promise/promise/README.md";
+  private static final String DEFAULT_URL = "https://raw.githubusercontent.com/iluwatar/java-design-patterns/Promise/promise/README.md";
   private ExecutorService executor;
-  private CountDownLatch canStop = new CountDownLatch(2);
-  
+  private CountDownLatch stopLatch = new CountDownLatch(2);
+
   private App() {
     executor = Executors.newFixedThreadPool(2);
   }
-  
+
   /**
    * Program entry point
    * @param args arguments
@@ -67,28 +77,25 @@ public class App {
   public static void main(String[] args) throws InterruptedException, ExecutionException {
     App app = new App();
     try {
-      app.run();
+      app.promiseUsage();
     } finally {
       app.stop();
     }
   }
 
-  private void run() throws InterruptedException, ExecutionException {
-    promiseUsage();
+  private void promiseUsage() {
+    calculateLineCount();
+
+    calculateLowestFrequencyChar();
   }
 
-  private void promiseUsage() {
-    
-    countLines()
-      .then(
-          count -> {
-            System.out.println("Line count is: " + count);
-            taskCompleted();
-          }
-      );
-    
-    lowestCharFrequency()
-      .then(
+  /*
+   * Calculate the lowest frequency character and when that promise is fulfilled,
+   * consume the result in a Consumer<Character>
+   */
+  private void calculateLowestFrequencyChar() {
+    lowestFrequencyChar()
+        .thenAccept(
           charFrequency -> {
             System.out.println("Char with lowest frequency is: " + charFrequency);
             taskCompleted();
@@ -96,49 +103,73 @@ public class App {
       );
   }
 
-  private Promise<Character> lowestCharFrequency() {
-    return characterFrequency()
-        .then(
-            charFrequency -> { 
-              return Utility.lowestFrequencyChar(charFrequency).orElse(null); 
-            }
-        );
-  }
-
-  private Promise<Map<Character, Integer>> characterFrequency() {
-    return download(URL)
-      .then(
-          fileLocation -> {
-            return Utility.characterFrequency(fileLocation);
+  /*
+   * Calculate the line count and when that promise is fulfilled, consume the result
+   * in a Consumer<Integer>
+   */
+  private void calculateLineCount() {
+    countLines()
+        .thenAccept(
+          count -> {
+            System.out.println("Line count is: " + count);
+            taskCompleted();
           }
       );
   }
 
-  private Promise<Integer> countLines() {
-    return download(URL)
-        .then(
-            fileLocation -> {
-              return Utility.countLines(fileLocation);
-            }
-        );
+  /*
+   * Calculate the character frequency of a file and when that promise is fulfilled,
+   * then promise to apply function to calculate lowest character frequency.
+   */
+  private Promise<Character> lowestFrequencyChar() {
+    return characterFrequency()
+        .thenApply(Utility::lowestFrequencyChar);
   }
 
+  /*
+   * Download the file at DEFAULT_URL and when that promise is fulfilled,
+   * then promise to apply function to calculate character frequency.
+   */
+  private Promise<Map<Character, Integer>> characterFrequency() {
+    return download(DEFAULT_URL)
+        .thenApply(Utility::characterFrequency);
+  }
+
+  /*
+   * Download the file at DEFAULT_URL and when that promise is fulfilled,
+   * then promise to apply function to count lines in that file.
+   */
+  private Promise<Integer> countLines() {
+    return download(DEFAULT_URL)
+        .thenApply(Utility::countLines);
+  }
+
+  /*
+   * Return a promise to provide the local absolute path of the file downloaded in background.
+   * This is an async method and does not wait until the file is downloaded.
+   */
   private Promise<String> download(String urlString) {
     Promise<String> downloadPromise = new Promise<String>()
         .fulfillInAsync(
             () -> {
               return Utility.downloadFile(urlString);
-            }, executor);
-    
+            }, executor)
+        .onError(
+            throwable -> {
+              throwable.printStackTrace();
+              taskCompleted();
+            }
+        );
+
     return downloadPromise;
   }
 
   private void stop() throws InterruptedException {
-    canStop.await();
+    stopLatch.await();
     executor.shutdownNow();
   }
-  
+
   private void taskCompleted() {
-    canStop.countDown();
+    stopLatch.countDown();
   }
 }
diff --git a/promise/src/main/java/com/iluwatar/promise/Promise.java b/promise/src/main/java/com/iluwatar/promise/Promise.java
index 7d8a97e84..870e1556d 100644
--- a/promise/src/main/java/com/iluwatar/promise/Promise.java
+++ b/promise/src/main/java/com/iluwatar/promise/Promise.java
@@ -36,6 +36,7 @@ import java.util.function.Function;
 public class Promise<T> extends PromiseSupport<T> {
 
   private Runnable fulfillmentAction;
+  private Consumer<? super Throwable> exceptionHandler;
 
   /**
    * Creates a promise that will be fulfilled in future.
@@ -61,9 +62,17 @@ public class Promise<T> extends PromiseSupport<T> {
   @Override
   public void fulfillExceptionally(Exception exception) {
     super.fulfillExceptionally(exception);
+    handleException(exception);
     postFulfillment();
   }
 
+  private void handleException(Exception exception) {
+    if (exceptionHandler == null) {
+      return;
+    }
+    exceptionHandler.accept(exception);
+  }
+
   private void postFulfillment() {
     if (fulfillmentAction == null) {
       return;
@@ -83,8 +92,8 @@ public class Promise<T> extends PromiseSupport<T> {
     executor.execute(() -> {
       try {
         fulfill(task.call());
-      } catch (Exception e) {
-        fulfillExceptionally(e);
+      } catch (Exception ex) {
+        fulfillExceptionally(ex);
       }
     });
     return this;
@@ -96,11 +105,22 @@ public class Promise<T> extends PromiseSupport<T> {
    * @param action action to be executed.
    * @return a new promise.
    */
-  public Promise<Void> then(Consumer<? super T> action) {
+  public Promise<Void> thenAccept(Consumer<? super T> action) {
     Promise<Void> dest = new Promise<>();
     fulfillmentAction = new ConsumeAction(this, dest, action);
     return dest;
   }
+  
+  /**
+   * Set the exception handler on this promise.
+   * @param exceptionHandler a consumer that will handle the exception occurred while fulfilling
+   *            the promise.
+   * @return this
+   */
+  public Promise<T> onError(Consumer<? super Throwable> exceptionHandler) {
+    this.exceptionHandler = exceptionHandler;
+    return this;
+  }
 
   /**
    * Returns a new promise that, when this promise is fulfilled normally, is fulfilled with 
@@ -108,7 +128,7 @@ public class Promise<T> extends PromiseSupport<T> {
    * @param func function to be executed.
    * @return a new promise.
    */
-  public <V> Promise<V> then(Function<? super T, V> func) {
+  public <V> Promise<V> thenApply(Function<? super T, V> func) {
     Promise<V> dest = new Promise<>();
     fulfillmentAction = new TransformAction<V>(this, dest, func);
     return dest;
@@ -135,8 +155,8 @@ public class Promise<T> extends PromiseSupport<T> {
       try {
         action.accept(src.get());
         dest.fulfill(null);
-      } catch (Throwable e) {
-        dest.fulfillExceptionally((Exception) e.getCause());
+      } catch (Throwable throwable) {
+        dest.fulfillExceptionally((Exception) throwable.getCause());
       }
     }
   }
@@ -162,8 +182,8 @@ public class Promise<T> extends PromiseSupport<T> {
       try {
         V result = func.apply(src.get());
         dest.fulfill(result);
-      } catch (Throwable e) {
-        dest.fulfillExceptionally((Exception) e.getCause());
+      } catch (Throwable throwable) {
+        dest.fulfillExceptionally((Exception) throwable.getCause());
       }
     }
   }
diff --git a/promise/src/main/java/com/iluwatar/promise/Utility.java b/promise/src/main/java/com/iluwatar/promise/Utility.java
index 2cfad46d0..8d5be2538 100644
--- a/promise/src/main/java/com/iluwatar/promise/Utility.java
+++ b/promise/src/main/java/com/iluwatar/promise/Utility.java
@@ -12,15 +12,19 @@ import java.net.URL;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Map.Entry;
 
 public class Utility {
 
+  /**
+   * Calculates character frequency of the file provided.
+   * @param fileLocation location of the file.
+   * @return a map of character to its frequency, an empty map if file does not exist.
+   */
   public static Map<Character, Integer> characterFrequency(String fileLocation) {
     Map<Character, Integer> characterToFrequency = new HashMap<>();
-    try (Reader reader = new FileReader(fileLocation); 
-        BufferedReader bufferedReader = new BufferedReader(reader);) {
+    try (Reader reader = new FileReader(fileLocation);
+        BufferedReader bufferedReader = new BufferedReader(reader)) {
       for (String line; (line = bufferedReader.readLine()) != null;) {
         for (char c : line.toCharArray()) {
           if (!characterToFrequency.containsKey(c)) {
@@ -35,33 +39,35 @@ public class Utility {
     }
     return characterToFrequency;
   }
-  
-  public static Optional<Character> lowestFrequencyChar(Map<Character, Integer> charFrequency) {
-    Optional<Character> lowestFrequencyChar = Optional.empty();
-    if (charFrequency.isEmpty()) {
-      return lowestFrequencyChar;
-    }
-    
+
+  /**
+   * @return the character with lowest frequency if it exists, {@code Optional.empty()} otherwise.
+   */
+  public static Character lowestFrequencyChar(Map<Character, Integer> charFrequency) {
+    Character lowestFrequencyChar = null;
     Iterator<Entry<Character, Integer>> iterator = charFrequency.entrySet().iterator();
     Entry<Character, Integer> entry = iterator.next();
     int minFrequency = entry.getValue();
-    lowestFrequencyChar = Optional.of(entry.getKey());
-    
+    lowestFrequencyChar = entry.getKey();
+
     while (iterator.hasNext()) {
       entry = iterator.next();
       if (entry.getValue() < minFrequency) {
         minFrequency = entry.getValue();
-        lowestFrequencyChar = Optional.of(entry.getKey());
+        lowestFrequencyChar = entry.getKey();
       }
     }
-    
+
     return lowestFrequencyChar;
   }
-  
+
+  /**
+   * @return number of lines in the file at provided location. 0 if file does not exist.
+   */
   public static Integer countLines(String fileLocation) {
     int lineCount = 0;
-    try (Reader reader = new FileReader(fileLocation); 
-        BufferedReader bufferedReader = new BufferedReader(reader);) {
+    try (Reader reader = new FileReader(fileLocation);
+        BufferedReader bufferedReader = new BufferedReader(reader)) {
       while (bufferedReader.readLine() != null) {
         lineCount++;
       }
@@ -71,11 +77,15 @@ public class Utility {
     return lineCount;
   }
 
+  /**
+   * Downloads the contents from the given urlString, and stores it in a temporary directory.
+   * @return the absolute path of the file downloaded.
+   */
   public static String downloadFile(String urlString) throws MalformedURLException, IOException {
     System.out.println("Downloading contents from url: " + urlString);
     URL url = new URL(urlString);
     File file = File.createTempFile("promise_pattern", null);
-    try (Reader reader = new InputStreamReader(url.openStream()); 
+    try (Reader reader = new InputStreamReader(url.openStream());
         BufferedReader bufferedReader = new BufferedReader(reader);
         FileWriter writer = new FileWriter(file)) {
       for (String line; (line = bufferedReader.readLine()) != null; ) {
diff --git a/promise/src/test/java/com/iluwatar/promise/PromiseTest.java b/promise/src/test/java/com/iluwatar/promise/PromiseTest.java
index de0ecb6d7..45c4c1d36 100644
--- a/promise/src/test/java/com/iluwatar/promise/PromiseTest.java
+++ b/promise/src/test/java/com/iluwatar/promise/PromiseTest.java
@@ -26,6 +26,9 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
@@ -40,7 +43,6 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
-
 /**
  * Tests Promise class.
  */
@@ -73,7 +75,8 @@ public class PromiseTest {
     testWaitingSomeTimeForPromiseToBeFulfilled();
   }
 
-  private void testWaitingForeverForPromiseToBeFulfilled() throws InterruptedException, TimeoutException {
+  private void testWaitingForeverForPromiseToBeFulfilled() 
+      throws InterruptedException, TimeoutException {
     Promise<Integer> promise = new Promise<>();
     promise.fulfillInAsync(new Callable<Integer>() {
 
@@ -134,7 +137,7 @@ public class PromiseTest {
       throws InterruptedException, ExecutionException {
     Promise<Void> dependentPromise = promise
         .fulfillInAsync(new NumberCrunchingTask(), executor)
-        .then(value -> {
+        .thenAccept(value -> {
           assertEquals(NumberCrunchingTask.CRUNCHED_NUMBER, value);
         });
 
@@ -149,17 +152,18 @@ public class PromiseTest {
       throws InterruptedException, ExecutionException, TimeoutException {
     Promise<Void> dependentPromise = promise
         .fulfillInAsync(new NumberCrunchingTask(), executor)
-        .then(new Consumer<Integer>() {
+        .thenAccept(new Consumer<Integer>() {
 
           @Override
-          public void accept(Integer t) {
+          public void accept(Integer value) {
             throw new RuntimeException("Barf!");
           }
         });
 
     try {
       dependentPromise.get();
-      fail("Fetching dependent promise should result in exception if the action threw an exception");
+      fail("Fetching dependent promise should result in exception "
+          + "if the action threw an exception");
     } catch (ExecutionException ex) {
       assertTrue(promise.isDone());
       assertFalse(promise.isCancelled());
@@ -167,7 +171,8 @@ public class PromiseTest {
     
     try {
       dependentPromise.get(1000, TimeUnit.SECONDS);
-      fail("Fetching dependent promise should result in exception if the action threw an exception");
+      fail("Fetching dependent promise should result in exception "
+          + "if the action threw an exception");
     } catch (ExecutionException ex) {
       assertTrue(promise.isDone());
       assertFalse(promise.isCancelled());
@@ -179,7 +184,7 @@ public class PromiseTest {
       throws InterruptedException, ExecutionException {
     Promise<String> dependentPromise = promise
         .fulfillInAsync(new NumberCrunchingTask(), executor)
-        .then(value -> {
+        .thenApply(value -> {
           assertEquals(NumberCrunchingTask.CRUNCHED_NUMBER, value); 
           return String.valueOf(value);
         });
@@ -195,17 +200,18 @@ public class PromiseTest {
       throws InterruptedException, ExecutionException, TimeoutException {
     Promise<String> dependentPromise = promise
         .fulfillInAsync(new NumberCrunchingTask(), executor)
-        .then(new Function<Integer, String>() {
+        .thenApply(new Function<Integer, String>() {
 
           @Override
-          public String apply(Integer t) {
+          public String apply(Integer value) {
             throw new RuntimeException("Barf!");
           }
         });
 
     try {
       dependentPromise.get();
-      fail("Fetching dependent promise should result in exception if the function threw an exception");
+      fail("Fetching dependent promise should result in exception "
+          + "if the function threw an exception");
     } catch (ExecutionException ex) {
       assertTrue(promise.isDone());
       assertFalse(promise.isCancelled());
@@ -213,7 +219,8 @@ public class PromiseTest {
     
     try {
       dependentPromise.get(1000, TimeUnit.SECONDS);
-      fail("Fetching dependent promise should result in exception if the function threw an exception");
+      fail("Fetching dependent promise should result in exception "
+          + "if the function threw an exception");
     } catch (ExecutionException ex) {
       assertTrue(promise.isDone());
       assertFalse(promise.isCancelled());
@@ -228,6 +235,19 @@ public class PromiseTest {
     
     promise.get(1000, TimeUnit.SECONDS);
   }
+  
+  @SuppressWarnings("unchecked")
+  @Test
+  public void exceptionHandlerIsCalledWhenPromiseIsFulfilledExceptionally() {
+    Promise<Object> promise = new Promise<>();
+    Consumer<Throwable> exceptionHandler = mock(Consumer.class);
+    promise.onError(exceptionHandler);
+    
+    Exception exception = new Exception("barf!");
+    promise.fulfillExceptionally(exception);
+    
+    verify(exceptionHandler).accept(eq(exception));
+  }
 
   private static class NumberCrunchingTask implements Callable<Integer> {
 
@@ -236,7 +256,7 @@ public class PromiseTest {
     @Override
     public Integer call() throws Exception {
       // Do number crunching
-      Thread.sleep(1000);
+      Thread.sleep(100);
       return CRUNCHED_NUMBER;
     }
   }