From 339dacc01c4f4c63a43421c6814edf460f94cda1 Mon Sep 17 00:00:00 2001 From: Narendra Pathai <narendra.pathai@gmail.com> Date: Mon, 15 Oct 2018 21:16:00 +0530 Subject: [PATCH] Addressing review comments, Deleting unintentional file and used FAILURE constant in ClientTest as well --- ambassador.patch | 205 ------------------ .../com/iluwatar/ambassador/ClientTest.java | 2 +- 2 files changed, 1 insertion(+), 206 deletions(-) delete mode 100644 ambassador.patch diff --git a/ambassador.patch b/ambassador.patch deleted file mode 100644 index a8e4bfeb9..000000000 --- a/ambassador.patch +++ /dev/null @@ -1,205 +0,0 @@ -diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java b/ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java -index 225e033..52b792b 100644 ---- a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteService.java -+++ b/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 @@ import static java.lang.Thread.sleep; - * 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 @@ public class RemoteService implements RemoteServiceInterface { - 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; - } - } -diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java b/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java -index 940313d..e228271 100644 ---- a/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java -+++ b/ambassador/src/main/java/com/iluwatar/ambassador/RemoteServiceInterface.java -@@ -26,6 +26,7 @@ package com.iluwatar.ambassador; - * Interface shared by ({@link RemoteService}) and ({@link ServiceAmbassador}). - */ - interface RemoteServiceInterface { -- -+ int FAILURE = -1; -+ - long doRemoteFunction(int value) throws Exception; - } -diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java b/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java -index 4be1dfc..52cd7df 100644 ---- a/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java -+++ b/ambassador/src/main/java/com/iluwatar/ambassador/ServiceAmbassador.java -@@ -59,15 +59,15 @@ public class ServiceAmbassador implements RemoteServiceInterface { - 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 { -diff --git a/ambassador/src/main/java/com/iluwatar/ambassador/util/RandomProvider.java b/ambassador/src/main/java/com/iluwatar/ambassador/util/RandomProvider.java -new file mode 100644 -index 0000000..3afa3a6 ---- /dev/null -+++ b/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(); -+} -diff --git a/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java b/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java -index 9354c4f..1d69a6a 100644 ---- a/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java -+++ b/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java -@@ -24,6 +24,8 @@ package com.iluwatar.ambassador; - - import org.junit.jupiter.api.Test; - -+import static org.junit.jupiter.api.Assertions.assertTrue; -+ - /** - * Test for {@link Client} - */ -@@ -35,6 +37,6 @@ public class ClientTest { - Client client = new Client(); - long result = client.useService(10); - -- assert result == 100 || result == -1; -+ assertTrue(result == 100 || result == -1); - } - } -diff --git a/ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java b/ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java -index e0b297a..7d01469 100644 ---- a/ambassador/src/test/java/com/iluwatar/ambassador/RemoteServiceTest.java -+++ b/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; -+ } - } - } -diff --git a/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java b/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java -index 432df9f..72e7106 100644 ---- a/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java -+++ b/ambassador/src/test/java/com/iluwatar/ambassador/ServiceAmbassadorTest.java -@@ -24,6 +24,8 @@ package com.iluwatar.ambassador; - - 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); - } - } diff --git a/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java b/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java index 1d69a6a45..948ecaae8 100644 --- a/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java +++ b/ambassador/src/test/java/com/iluwatar/ambassador/ClientTest.java @@ -37,6 +37,6 @@ public class ClientTest { Client client = new Client(); long result = client.useService(10); - assertTrue(result == 100 || result == -1); + assertTrue(result == 100 || result == RemoteService.FAILURE); } }