refactor: Eliminate Optional from fields and parameters for async method invocation (#2830)

guqing · web-flow · commit 79dd5a7abcd4 · 2024-03-23T12:03:13.000+02:00
diff --git a/async-method-invocation/README.md b/async-method-invocation/README.md
@@ -57,7 +57,8 @@ public interface AsyncResult<T> {
 
 ```java
 public interface AsyncCallback<T> {
-  void onComplete(T value, Optional<Exception> ex);
+    void onComplete(T value);
+    void onError(Exception ex);
 }
 ```
 
diff --git a/async-method-invocation/etc/async-method-invocation.urm.puml b/async-method-invocation/etc/async-method-invocation.urm.puml
@@ -9,7 +9,8 @@ package com.iluwatar.async.method.invocation {
     + main(args : String[]) {static}
   }
   interface AsyncCallback<T> {
-    + onComplete(T, Optional<Exception>) {abstract}
+    + onComplete(T) {abstract}
+    + onError(Exception) {abstract}
   }
   interface AsyncExecutor {
     + endProcess(AsyncResult<T>) : T {abstract}
@@ -32,20 +33,23 @@ package com.iluwatar.async.method.invocation {
     ~ COMPLETED : int {static}
     ~ FAILED : int {static}
     ~ RUNNING : int {static}
-    ~ callback : Optional<AsyncCallback<T>>
+    ~ callback : AsyncCallback<T>
     ~ exception : Exception
     ~ lock : Object
     ~ state : int
     ~ value : T
     ~ CompletableResult<T>(callback : AsyncCallback<T>)
     + await()
     + getValue() : T
+    ~ hasCallback() : boolean
     + isCompleted() : boolean
     ~ setException(exception : Exception)
     ~ setValue(value : T)
   }
 }
+ThreadAsyncExecutor ..|> AsyncCallback
+ThreadAsyncExecutor ..|> AsyncResult
+ThreadAsyncExecutor ..|> AsyncExecutor
 CompletableResult ..+ ThreadAsyncExecutor
-ThreadAsyncExecutor ..|> AsyncExecutor 
-CompletableResult ..|> AsyncResult 
-@enduml
+CompletableResult ..|> AsyncResult
+@enduml
diff --git a/async-method-invocation/src/main/java/com/iluwatar/async/method/invocation/App.java b/async-method-invocation/src/main/java/com/iluwatar/async/method/invocation/App.java
@@ -118,12 +118,16 @@ private static <T> Callable<T> lazyval(T value, long delayMillis) {
    * @return new async callback
    */
   private static <T> AsyncCallback<T> callback(String name) {
-    return (value, ex) -> {
-      if (ex.isPresent()) {
-        log(name + " failed: " + ex.map(Exception::getMessage).orElse(""));
-      } else {
+    return new AsyncCallback<>() {
+      @Override
+      public void onComplete(T value) {
         log(name + " <" + value + ">");
       }
+
+      @Override
+      public void onError(Exception ex) {
+        log(name + " failed: " + ex.getMessage());
+      }
     };
   }
 
diff --git a/async-method-invocation/src/main/java/com/iluwatar/async/method/invocation/AsyncCallback.java b/async-method-invocation/src/main/java/com/iluwatar/async/method/invocation/AsyncCallback.java
@@ -24,8 +24,6 @@
  */
 package com.iluwatar.async.method.invocation;
 
-import java.util.Optional;
-
 /**
  * AsyncCallback interface.
  *
@@ -34,10 +32,16 @@
 public interface AsyncCallback<T> {
 
   /**
-   * Complete handler which is executed when async task is completed or fails execution.
+   * Complete handler which is executed when async task is completed.
+   *
+   * @param value the evaluated value from async task
+   */
+  void onComplete(T value);
+
+  /**
+   * Error handler which is executed when async task fails execution.
    *
-   * @param value the evaluated value from async task, undefined when execution fails
-   * @param ex    empty value if execution succeeds, some exception if executions fails
+   * @param ex exception which was thrown during async task execution(non-null)
    */
-  void onComplete(T value, Optional<Exception> ex);
+  void onError(Exception ex);
 }
diff --git a/async-method-invocation/src/main/java/com/iluwatar/async/method/invocation/ThreadAsyncExecutor.java b/async-method-invocation/src/main/java/com/iluwatar/async/method/invocation/ThreadAsyncExecutor.java
@@ -81,15 +81,19 @@ private static class CompletableResult<T> implements AsyncResult<T> {
     static final int COMPLETED = 3;
 
     final Object lock;
-    final Optional<AsyncCallback<T>> callback;
+    final AsyncCallback<T> callback;
 
     volatile int state = RUNNING;
     T value;
     Exception exception;
 
     CompletableResult(AsyncCallback<T> callback) {
       this.lock = new Object();
-      this.callback = Optional.ofNullable(callback);
+      this.callback = callback;
+    }
+
+    boolean hasCallback() {
+      return callback != null;
     }
 
     /**
@@ -101,7 +105,9 @@ private static class CompletableResult<T> implements AsyncResult<T> {
     void setValue(T value) {
       this.value = value;
       this.state = COMPLETED;
-      this.callback.ifPresent(ac -> ac.onComplete(value, Optional.empty()));
+      if (hasCallback()) {
+        callback.onComplete(value);
+      }
       synchronized (lock) {
         lock.notifyAll();
       }
@@ -116,7 +122,9 @@ void setValue(T value) {
     void setException(Exception exception) {
       this.exception = exception;
       this.state = FAILED;
-      this.callback.ifPresent(ac -> ac.onComplete(null, Optional.of(exception)));
+      if (hasCallback()) {
+        callback.onError(exception);
+      }
       synchronized (lock) {
         lock.notifyAll();
       }
diff --git a/async-method-invocation/src/test/java/com/iluwatar/async/method/invocation/ThreadAsyncExecutorTest.java b/async-method-invocation/src/test/java/com/iluwatar/async/method/invocation/ThreadAsyncExecutorTest.java
@@ -25,15 +25,8 @@
 package com.iluwatar.async.method.invocation;
 
 import static java.time.Duration.ofMillis;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertSame;
-import static org.junit.jupiter.api.Assertions.assertTimeout;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.ArgumentMatchers.isNull;
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -58,7 +51,7 @@
 class ThreadAsyncExecutorTest {
 
   @Captor
-  private ArgumentCaptor<Optional<Exception>> optionalCaptor;
+  private ArgumentCaptor<Exception> exceptionCaptor;
 
   @Mock
   private Callable<Object> task;
@@ -118,11 +111,8 @@ void testSuccessfulTaskWithCallback() {
       verify(task, times(1)).call();
 
       // ... same for the callback, we expect our object
-      verify(callback, times(1)).onComplete(eq(result), optionalCaptor.capture());
-
-      final var optionalException = optionalCaptor.getValue();
-      assertNotNull(optionalException);
-      assertFalse(optionalException.isPresent());
+      verify(callback, times(1)).onComplete(eq(result));
+      verify(callback, times(0)).onError(exceptionCaptor.capture());
 
       // ... and the result should be exactly the same object
       assertSame(result, asyncResult.getValue());
@@ -200,11 +190,8 @@ void testLongRunningTaskWithCallback() {
 
       // Our task should only execute once, but it can take a while ...
       verify(task, timeout(3000).times(1)).call();
-      verify(callback, timeout(3000).times(1)).onComplete(eq(result), optionalCaptor.capture());
-
-      final var optionalException = optionalCaptor.getValue();
-      assertNotNull(optionalException);
-      assertFalse(optionalException.isPresent());
+      verify(callback, timeout(3000).times(1)).onComplete(eq(result));
+      verify(callback, times(0)).onError(isA(Exception.class));
 
       // Prevent timing issues, and wait until the result is available
       asyncResult.await();
@@ -295,14 +282,12 @@ void testNullTaskWithCallback() {
       assertNotNull(asyncResult, "The AsyncResult should not be 'null', even though the task was 'null'.");
       asyncResult.await(); // Prevent timing issues, and wait until the result is available
       assertTrue(asyncResult.isCompleted());
-      verify(callback, times(1)).onComplete(isNull(), optionalCaptor.capture());
+      verify(callback, times(0)).onComplete(any());
+      verify(callback, times(1)).onError(exceptionCaptor.capture());
 
-      final var optionalException = optionalCaptor.getValue();
-      assertNotNull(optionalException);
-      assertTrue(optionalException.isPresent());
-
-      final var exception = optionalException.get();
+      final var exception = exceptionCaptor.getValue();
       assertNotNull(exception);
+
       assertEquals(NullPointerException.class, exception.getClass());
 
       try {
@@ -347,4 +332,4 @@ void testNullTaskWithNullCallback() {
 
   }
 
-}
+}

Original file line number	Diff line number	Diff line change
`@@ -57,7 +57,8 @@ public interface AsyncResult<T> {`
`57`	`57`
`58`	`58`	```java
`59`	`59`	`public interface AsyncCallback<T> {`
`60`		`- void onComplete(T value, Optional<Exception> ex);`
	`60`	`+ void onComplete(T value);`
	`61`	`+ void onError(Exception ex);`
`61`	`62`	`}`
`62`	`63`	```
`63`	`64`