Refactoring changes in Ambassador Pattern (iluwatar#805)

* 1) Updated test cases to use Junit Assert method as compared to assert keyword 2) Proper testing of RemoteService using RandomProvider interface. Introduced RandomProvider interface so that randomness can be controlled from test cases. 3) For readability used constant for representing FAILURE

* Addressing review comments, Deleting unintentional file and used FAILURE constant in ClientTest as well

Author: npathai

ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java

@@ -22,6 +22,7 @@
  */
 package com.iluwatar.ambassador;
 
+import com.iluwatar.ambassador.util.RandomProvider;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -31,9 +32,10 @@
  * A remote legacy application represented by a Singleton implementation.
  */
 public class RemoteService implements RemoteServiceInterface {
-
+  static final int THRESHOLD = 200;
   private static final Logger LOGGER = LoggerFactory.getLogger(RemoteService.class);
   private static RemoteService service = null;
+  private final RandomProvider randomProvider;
 
   static synchronized RemoteService getRemoteService() {
     if (service == null) {
@@ -42,24 +44,33 @@ static synchronized RemoteService getRemoteService() {
     return service;
   }
 
-  private RemoteService() {}
+  private RemoteService() {
+    this(Math::random);
+  }
 
+  /**
+   * This constuctor is used for testing purposes only.
+   */
+  RemoteService(RandomProvider randomProvider) {
+    this.randomProvider = randomProvider;
+  }
   /**
    * Remote function takes a value and multiplies it by 10 taking a random amount of time.
    * Will sometimes return -1. This imitates connectivity issues a client might have to account for.
    * @param value integer value to be multiplied.
-   * @return if waitTime is more than 200ms, it returns value * 10, otherwise -1.
+   * @return if waitTime is less than {@link RemoteService#THRESHOLD}, it returns value * 10,
+   *     otherwise {@link RemoteServiceInterface#FAILURE}.
    */
   @Override
   public long doRemoteFunction(int value) {
 
-    long waitTime = (long) Math.floor(Math.random() * 1000);
+    long waitTime = (long) Math.floor(randomProvider.random() * 1000);
 
     try {
       sleep(waitTime);
     } catch (InterruptedException e) {
       LOGGER.error("Thread sleep state interrupted", e);
     }
-    return waitTime >= 200 ? value * 10 : -1;
+    return waitTime <= THRESHOLD ? value * 10 : FAILURE;
   }
 }

ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java

@@ -26,6 +26,7 @@
  * Interface shared by ({@link RemoteService}) and ({@link ServiceAmbassador}).
  */
 interface RemoteServiceInterface {
-    
+  int FAILURE = -1;
+
   long doRemoteFunction(int value) throws Exception;
 }

ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java

@@ -59,15 +59,15 @@ private long checkLatency(int value) {
   private long safeCall(int value) {
 
     int retries = 0;
-    long result = -1;
+    long result = FAILURE;
 
     for (int i = 0; i < RETRIES; i++) {
 
       if (retries >= RETRIES) {
-        return -1;
+        return FAILURE;
       }
 
-      if ((result = checkLatency(value)) == -1) {
+      if ((result = checkLatency(value)) == FAILURE) {
         LOGGER.info("Failed to reach remote: (" + (i + 1) + ")");
         retries++;
         try {

ambassador/src/main/java/com/iluwatar/ambassador/util/RandomProvider.java

@@ -0,0 +1,8 @@
+package com.iluwatar.ambassador.util;
+
+/**
+ * An interface for randomness. Useful for testing purposes.
+ */
+public interface RandomProvider {
+  double random();
+}

ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java

@@ -24,6 +24,8 @@
 
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 /**
  * Test for {@link Client}
  */
@@ -35,6 +37,6 @@ public void test() {
     Client client = new Client();
     long result = client.useService(10);
 
-    assert result == 100 || result == -1;
+    assertTrue(result == 100 || result == RemoteService.FAILURE);
   }
 }

ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java

@@ -22,16 +22,43 @@
  */
 package com.iluwatar.ambassador;
 
+import com.iluwatar.ambassador.util.RandomProvider;
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 /**
  * Test for {@link RemoteService}
  */
 public class RemoteServiceTest {
 
   @Test
-  public void test() {
-    long result = RemoteService.getRemoteService().doRemoteFunction(10);
-    assert result == 100 || result == -1;
+  public void testFailedCall() {
+    RemoteService remoteService = new RemoteService(
+        new StaticRandomProvider(0.21));
+    long result = remoteService.doRemoteFunction(10);
+    assertEquals(RemoteServiceInterface.FAILURE, result);
+  }
+
+  @Test
+  public void testSuccessfulCall() {
+    RemoteService remoteService = new RemoteService(
+        new StaticRandomProvider(0.2));
+    long result = remoteService.doRemoteFunction(10);
+    assertEquals(100, result);
+  }
+
+  private class StaticRandomProvider implements RandomProvider {
+    private double value;
+
+    StaticRandomProvider(double value) {
+      this.value = value;
+    }
+
+    @Override
+    public double random() {
+      return value;
+    }
   }
 }

ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java

@@ -24,6 +24,8 @@
 
 import org.junit.jupiter.api.Test;
 
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
 /**
  * Test for {@link ServiceAmbassador}
  */
@@ -32,6 +34,6 @@ public class ServiceAmbassadorTest {
   @Test
   public void test() {
     long result = new ServiceAmbassador().doRemoteFunction(10);
-    assert result == 100 || result == -1;
+    assertTrue(result == 100 || result == RemoteServiceInterface.FAILURE);
   }
 }