From 9bbb4da3d5a58c666f186bbe45f5ccf599f63120 Mon Sep 17 00:00:00 2001 From: Narendra Pathai Date: Tue, 16 Oct 2018 12:15:39 +0530 Subject: [PATCH] 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 --- .../java/com/iluwatar/event/queue/App.java | 14 +++-- .../java/com/iluwatar/event/queue/Audio.java | 59 +++++++++++-------- .../com/iluwatar/event/queue/AudioTest.java | 29 ++++++--- 3 files changed, 64 insertions(+), 38 deletions(-) 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 index ea107d6ca..155be9c74 100644 --- 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 index 6534b563e..d059ccac0 100644 --- 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 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 +108,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 +128,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; @@ -143,6 +148,7 @@ public class Audio { e.printStackTrace(); } catch (IllegalArgumentException e) { System.err.println("The system doesn't support the sound: " + e.getMessage()); + e.printStackTrace(); } } @@ -153,7 +159,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,8 +168,15 @@ public class Audio { * 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 index 47f332526..8cd7c9b89 100644 --- 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 @@ 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()); } }