1) Test cases were not stopping AudioService before ending test case 2) Changed Audio to be a good singleton, previously because of Audio being bad singleton, test cases which were using static methods could have caused intermittent failures. 3) Made some other refactorings as well

npathai · npathai · commit 9bbb4da3d5a5 · 2018-10-16T12:15:39.000+05:30
diff --git a/event-queue/src/main/java/com/iluwatar/event/queue/App.java b/event-queue/src/main/java/com/iluwatar/event/queue/App.java
@@ -47,13 +47,15 @@ public class App {
    * @throws IOException when there is a problem with the audio file loading
    * @throws UnsupportedAudioFileException  when the loaded audio file is unsupported 
    */
-  public static void main(String[] args) throws UnsupportedAudioFileException, IOException {
-    Audio.playSound(Audio.getAudioStream("./etc/Bass-Drum-1.wav"), -10.0f);
-    Audio.playSound(Audio.getAudioStream("./etc/Closed-Hi-Hat-1.wav"), -8.0f);
+  public static void main(String[] args) throws UnsupportedAudioFileException, IOException, InterruptedException {
+    Audio audio = Audio.getInstance();
+    audio.playSound(audio.getAudioStream("./etc/Bass-Drum-1.wav"), -10.0f);
+    audio.playSound(audio.getAudioStream("./etc/Closed-Hi-Hat-1.wav"), -8.0f);
     
     System.out.println("Press Enter key to stop the program...");
-    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
-    br.read();
-    Audio.stopService();
+    try (BufferedReader br = new BufferedReader(new InputStreamReader(System.in))) {
+      br.read();
+    }
+    audio.stopService();
   }
 }
diff --git a/event-queue/src/main/java/com/iluwatar/event/queue/Audio.java b/event-queue/src/main/java/com/iluwatar/event/queue/Audio.java
@@ -41,46 +41,51 @@ public class Audio {
 
   private static final int MAX_PENDING = 16;
 
-  private static int headIndex;
+  private int headIndex;
 
-  private static int tailIndex;
+  private int tailIndex;
 
-  private static Thread updateThread = null;
+  private volatile Thread updateThread = null;
 
-  private static PlayMessage[] pendingAudio = new PlayMessage[MAX_PENDING];
+  private PlayMessage[] pendingAudio = new PlayMessage[MAX_PENDING];
+
+  // Visible only for testing purposes
+  Audio() {
+
+  }
+
+  public static Audio getInstance() {
+    return SingletonHolder.getAudioInstance();
+  }
 
   /**
-   * This method stops the Update Method's thread. 
+   * This method stops the Update Method's thread and waits till service stops.
    */
-  public static synchronized void stopService() {
+  public synchronized void stopService() throws InterruptedException {
     if (updateThread != null) {
       updateThread.interrupt();
     }
+    updateThread.join();
+    updateThread = null;
   }
   
   /**
    * This method check the Update Method's thread is started.
    * @return boolean
    */
-  public static synchronized boolean isServiceRunning() {
-    if (updateThread != null && updateThread.isAlive() ) {
-      return true;
-    } else {
-      return false;
-    }
+  public synchronized boolean isServiceRunning() {
+    return updateThread != null && updateThread.isAlive();
   }
 
   /**
    * Starts the thread for the Update Method pattern if it was not started previously.
    * Also when the thread is is ready initializes the indexes of the queue 
    */
-  public static void init() {
+  public void init() {
     if (updateThread == null) {
-      updateThread = new Thread(new Runnable() {
-        public void run() {
-          while (!Thread.currentThread().isInterrupted()) {
-            Audio.update();
-          }
+      updateThread = new Thread(() -> {
+        while (!Thread.currentThread().isInterrupted()) {
+          update();
         }
       });
     }
@@ -90,7 +95,7 @@ public void run() {
   /**
    * This is a synchronized thread starter
    */
-  public static synchronized void startThread() {
+  private synchronized void startThread() {
     if (!updateThread.isAlive()) {
       updateThread.start();
       headIndex = 0;
@@ -103,7 +108,7 @@ public static synchronized void startThread() {
    * @param stream is the AudioInputStream for the method
    * @param volume is the level of the audio's volume 
    */
-  public static void playSound(AudioInputStream stream, float volume) {
+  public void playSound(AudioInputStream stream, float volume) {
     init();
     // Walk the pending requests.
     for (int i = headIndex; i != tailIndex; i = (i + 1) % MAX_PENDING) {
@@ -123,7 +128,7 @@ public static void playSound(AudioInputStream stream, float volume) {
    * This method uses the Update Method pattern.
    * It takes the audio from the queue and plays it
    */
-  public static void update() {
+  private void update() {
     // If there are no pending requests, do nothing.
     if (headIndex == tailIndex) {
       return;
@@ -143,6 +148,7 @@ public static void update() {
       e.printStackTrace();
     } catch (IllegalArgumentException e) {
       System.err.println("The system doesn't support the sound: " + e.getMessage());
+      e.printStackTrace();
     }
   }
 
@@ -153,7 +159,7 @@ public static void update() {
    * @throws UnsupportedAudioFileException when the audio file is not supported 
    * @throws IOException when the file is not readable
    */
-  public static AudioInputStream getAudioStream(String filePath) 
+  public AudioInputStream getAudioStream(String filePath)
       throws UnsupportedAudioFileException, IOException {
     return AudioSystem.getAudioInputStream(new File(filePath).getAbsoluteFile());
   }
@@ -162,8 +168,15 @@ public static AudioInputStream getAudioStream(String filePath)
    * Returns with the message array of the queue 
    * @return PlayMessage[]
    */
-  public static PlayMessage[] getPendingAudio() {
+  public PlayMessage[] getPendingAudio() {
     return pendingAudio;
   }
 
+  private static class SingletonHolder {
+    private static final Audio INSTANCE = new Audio();
+
+    static Audio getAudioInstance() {
+      return INSTANCE;
+    }
+  }
 }
diff --git a/event-queue/src/test/java/com/iluwatar/event/queue/AudioTest.java b/event-queue/src/test/java/com/iluwatar/event/queue/AudioTest.java
@@ -23,6 +23,7 @@
 
 package com.iluwatar.event.queue;
 
+import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import javax.sound.sampled.UnsupportedAudioFileException;
@@ -39,6 +40,12 @@
  */
 public class AudioTest {
 
+  private Audio audio;
+
+  @BeforeEach
+  void createAudioInstance() {
+    audio = new Audio();
+  }
   /**
    * Test here that the playSound method works correctly
    * @throws UnsupportedAudioFileException when the audio file is not supported 
@@ -47,13 +54,15 @@ public class AudioTest {
    */
   @Test
   public void testPlaySound() throws UnsupportedAudioFileException, IOException, InterruptedException {
-    Audio.playSound(Audio.getAudioStream("./etc/Bass-Drum-1.wav"), -10.0f);
+    audio.playSound(audio.getAudioStream("./etc/Bass-Drum-1.wav"), -10.0f);
     // test that service is started
-    assertTrue(Audio.isServiceRunning());
+    assertTrue(audio.isServiceRunning());
     // adding a small pause to be sure that the sound is ended
     Thread.sleep(5000);
+
+    audio.stopService();
     // test that service is finished
-    assertFalse(!Audio.isServiceRunning());
+    assertFalse(audio.isServiceRunning());
   }
 
   /**
@@ -64,16 +73,18 @@ public void testPlaySound() throws UnsupportedAudioFileException, IOException, I
    */
   @Test
   public void testQueue() throws UnsupportedAudioFileException, IOException, InterruptedException {
-    Audio.playSound(Audio.getAudioStream("./etc/Bass-Drum-1.aif"), -10.0f);
-    Audio.playSound(Audio.getAudioStream("./etc/Bass-Drum-1.aif"), -10.0f);
-    Audio.playSound(Audio.getAudioStream("./etc/Bass-Drum-1.aif"), -10.0f);
-    assertTrue(Audio.getPendingAudio().length > 0);
+    audio.playSound(audio.getAudioStream("./etc/Bass-Drum-1.aif"), -10.0f);
+    audio.playSound(audio.getAudioStream("./etc/Bass-Drum-1.aif"), -10.0f);
+    audio.playSound(audio.getAudioStream("./etc/Bass-Drum-1.aif"), -10.0f);
+    assertTrue(audio.getPendingAudio().length > 0);
     // test that service is started
-    assertTrue(Audio.isServiceRunning());
+    assertTrue(audio.isServiceRunning());
     // adding a small pause to be sure that the sound is ended
     Thread.sleep(10000);
+
+    audio.stopService();
     // test that service is finished
-    assertFalse(!Audio.isServiceRunning());
+    assertFalse(audio.isServiceRunning());
   }
 
 }
