I have a program I made a while a back for a friend and I that functions as a timer and alarm. It plays a sound and shows an image when you start the timer, and plays another sound and image when the timer ends. It has been working fine (more or less), but my friend recently switched to Linux and has some much more interactive sound controllers that led to us discovering an issue with the program: each time a sound plays, it creates an entirely new input stream that doesn't get reused and won't go away until the program is closed entirely. Not only is this poor design on my part that I want to learn how to fix, but it also causes issues on my friend's end when he tries to use the timer multiple times without closing the program. I believe the fix is to make it so that all sound plays through one input stream (and if anyone knows more about the interactions of Java and Linux please correct me). I tried simply making the AudioInputStream a global variable and resetting it before each use, but this causes both the image and sound to quit working entirely, and I have no idea why. Below is the actionPerformed on button press from my current and defective code. Following that is the working but ineffective code.
private void buttonActionPerformed(java.awt.event.ActionEvent evt) {
boolean needNewThread = false;
if (imgWindow.getIcon() == null) {
needNewThread = true;
}
timeUpLabel.setText(" ");
BufferedImage buffImg = null;
try{
globalAudioIn.reset();
URL url = this.getClass().getResource("/EvilManiMani.wav");
globalAudioIn = AudioSystem.getAudioInputStream(url);
Clip clip = AudioSystem.getClip();
clip.open(globalAudioIn);
clip.start();
buffImg = ImageIO.read(getClass().getResource("/images/cloudWait.png"));
imgWindow.setIcon(new ImageIcon(buffImg));
System.out.println(buffImg.toString());
} catch (Exception e){
System.out.println(e.getMessage());
}
Integer inputTime = Integer.parseInt(timeTextField.getText());
timerLabel.setText(inputTime + ":00");
if(needNewThread) {
t = new Timer(1000, new ActionListener (){
@Override
public void actionPerformed(ActionEvent ae){
String[] minsAndSeconds = timerLabel.getText().split(":");
boolean timesUp = false;
if(minsAndSeconds[0].startsWith("-")) {
timesUp = true;
String temp = minsAndSeconds[0].substring(1);
minsAndSeconds[0] = temp;
}
Integer minutes = Integer.parseInt(minsAndSeconds[0]);
Integer seconds = Integer.parseInt(minsAndSeconds[1]);
seconds += (minutes*60);
if(seconds > 0 && !timesUp){
minutes = --seconds/60;
seconds %= 60;
if(seconds >= 10) {
timerLabel.setText(minutes + ":" + seconds);
}else {
timerLabel.setText(minutes + ":0" + seconds);
}
}else if(seconds > 0 && timesUp) {
minutes = ++seconds/60;
seconds %= 60;
if(seconds >= 10) {
timerLabel.setText("-" + minutes + ":" + seconds);
}else {
timerLabel.setText("-" + minutes + ":0" + seconds);
}
}else if (seconds == 0){
timerLabel.setText("-0:01");
BufferedImage bufferedImg = null;
try {
globalAudioIn.reset();
URL url = this.getClass().getResource("/YouWin!.wav");
globalAudioIn = AudioSystem.getAudioInputStream(url);
Clip clip = AudioSystem.getClip();
clip.open(globalAudioIn);
clip.start();
bufferedImg = ImageIO.read(getClass().getResource("/images/drinkyMattPog.png"));
imgWindow.setIcon(new ImageIcon(bufferedImg));
timeUpLabel.setText("Time's up");
} catch (Exception e) {
System.out.println(e.getMessage());
}
}
}
});
t.setRepeats(true);
t.start();
}
}
Working but ineffective:
private void buttonActionPerformed(java.awt.event.ActionEvent evt) {
boolean needNewThread = false;
if (imgWindow.getIcon() == null) {
needNewThread = true;
}
timeUpLabel.setText(" ");
BufferedImage buffImg = null;
try{
URL url = this.getClass().getResource("/EvilManiMani.wav");
AudioInputStream audioIn = AudioSystem.getAudioInputStream(url);
Clip clip = AudioSystem.getClip();
clip.open(audioIn);
clip.start();
buffImg = ImageIO.read(getClass().getResource("/images/cloudWait.png"));
imgWindow.setIcon(new ImageIcon(buffImg));
System.out.println(buffImg.toString());
} catch (Exception e){
System.out.println(e.getMessage());
}
Integer inputTime = Integer.parseInt(timeTextField.getText());
timerLabel.setText(inputTime + ":00");
if(needNewThread) {
t = new Timer(1000, new ActionListener (){
@Override
public void actionPerformed(ActionEvent ae){
String[] minsAndSeconds = timerLabel.getText().split(":");
boolean timesUp = false;
if(minsAndSeconds[0].startsWith("-")) {
timesUp = true;
String temp = minsAndSeconds[0].substring(1);
minsAndSeconds[0] = temp;
}
Integer minutes = Integer.parseInt(minsAndSeconds[0]);
Integer seconds = Integer.parseInt(minsAndSeconds[1]);
seconds += (minutes*60);
if(seconds > 0 && !timesUp){
minutes = --seconds/60;
seconds %= 60;
if(seconds >= 10) {
timerLabel.setText(minutes + ":" + seconds);
}else {
timerLabel.setText(minutes + ":0" + seconds);
}
}else if(seconds > 0 && timesUp) {
minutes = ++seconds/60;
seconds %= 60;
if(seconds >= 10) {
timerLabel.setText("-" + minutes + ":" + seconds);
}else {
timerLabel.setText("-" + minutes + ":0" + seconds);
}
}else if (seconds == 0){
timerLabel.setText("-0:01");
BufferedImage bufferedImg = null;
try {
URL url = this.getClass().getResource("/YouWin!.wav");
AudioInputStream audioIn = AudioSystem.getAudioInputStream(url);
Clip clip = AudioSystem.getClip();
clip.open(audioIn);
clip.start();
bufferedImg = ImageIO.read(getClass().getResource("/images/drinkyMattPog.png"));
imgWindow.setIcon(new ImageIcon(bufferedImg));
timeUpLabel.setText("Time's up");
} catch (Exception e) {
System.out.println(e.getMessage());
}
}
}
});
t.setRepeats(true);
t.start();
}
}
Thanks! Also, any and all constructive criticism on the code is welcome, whether it's relevant to the question or not.
Before you call clip.start(), add a listener in order to close the Clip and the AudioInputStream:
clip.open(audioIn);
clip.addLineListener(e -> {
if (e.getType().equals(LineEvent.Type.STOP)) {
clip.close();
try {
audioIn.close();
} catch (IOException e) {
System.err.println(e);
}
}
});
clip.start();
Since you have asked for constructive criticism: Never write catch (Exception). You don’t want to catch RuntimeException or its subclasses, because they usually indicate programmer mistakes which need to be exposed and fixed, not glossed over. In your case, you should use catch (LineUnavailableException | IOException e) instead.
Your catch block should not print e.getMessage(). Instead, use e.printStackTrace(), so if an exception occurs, you will know exactly what happened and where.