Refactored Event Queue (#806)

* 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

* Removed sonar issue and converted Audio to eager singleton for simplicity

* Updated class diagram PNG

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();
   }
 }

event-queue/src/main/java/com/iluwatar/event/queue/Audio.java

@@ -23,6 +23,9 @@
 
 package com.iluwatar.event.queue;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import java.io.File;
 import java.io.IOException;
 
@@ -38,49 +41,56 @@ import javax.sound.sampled.UnsupportedAudioFileException;
  *
  */
 public class Audio {
+  private static final Logger LOGGER = LoggerFactory.getLogger(Audio.class);
+  private static final Audio INSTANCE = new 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 INSTANCE;
+  }
 
   /**
-   * 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 +100,7 @@ public class Audio {
   /**
    * This is a synchronized thread starter
    */
-  public static synchronized void startThread() {
+  private synchronized void startThread() {
     if (!updateThread.isAlive()) {
       updateThread.start();
       headIndex = 0;
@@ -103,7 +113,7 @@ public class Audio {
    * @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 +133,7 @@ public class Audio {
    * 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;
@@ -136,13 +146,11 @@ public class Audio {
       clip.open(audioStream);
       clip.start();
     } catch (LineUnavailableException e) {
-      System.err.println("Error occoured while loading the audio: The line is unavailable");
-      e.printStackTrace();
+      LOGGER.trace("Error occoured while loading the audio: The line is unavailable", e);
     } catch (IOException e) {
-      System.err.println("Input/Output error while loading the audio");
-      e.printStackTrace();
+      LOGGER.trace("Input/Output error while loading the audio", e);
     } catch (IllegalArgumentException e) {
-      System.err.println("The system doesn't support the sound: " + e.getMessage());
+      LOGGER.trace("The system doesn't support the sound: " + e.getMessage(), e);
     }
   }
 
@@ -153,7 +161,7 @@ public class Audio {
    * @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,7 +170,7 @@ public class Audio {
    * Returns with the message array of the queue 
    * @return PlayMessage[]
    */
-  public static PlayMessage[] getPendingAudio() {
+  public PlayMessage[] getPendingAudio() {
     return pendingAudio;
   }
 

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 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 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 class AudioTest {
    */
   @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());
   }
 
 }