HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Keyboard music maker

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
musickeyboardmaker

Problem

I enjoy making silly programs every once in a while. This is one of them. It makes nice sounding notes that vary slightly depending on the key being entered into a TextArea. When you press enter, it plays back all the notes you have entered. That's pretty much all it does. I'm looking for constructive criticism regarding the coding style, and perhaps usability.

```
package boop;

import java.awt.TextArea;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.util.HashMap;
import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.SourceDataLine;
import javax.swing.JFrame;

public class Boop extends JFrame implements KeyListener {
private final TextArea textArea;

private class Noise implements Runnable {

private int noise[];
public boolean sound = true;
private final int freq;
private int ms = 100;
private int samples = (int) ((ms * SAMPLE_RATE) / 1000);
private byte[] output = new byte[samples];
private int i = 0;
private double period;

public Noise(int freq) {
this.freq = freq;
period = (double) SAMPLE_RATE / freq;
new Thread(this).start();
}

@Override
public void run() {
SourceDataLine line;
try {
line = AudioSystem.getSourceDataLine(af);
line.open(af, SAMPLE_RATE);
} catch (LineUnavailableException ex) {
ex.printStackTrace();
return;
}
line.start();
while (i noises;

public static void main(String[] args) {
new Boop();
}

public Boop() {
noises = new HashMap<>();
textArea = new TextArea(20, 80);
textArea.addKeyListener(this);
add(textArea);
pack();
setVisible(true);
setDefaultCloseOper

Solution

Key-to-frequency mapping

Characters with an ASCII code that is a multiple of 20 (such as d) are mapped to 0 Hz, resulting in silence. You wrote new Noise(c % 20 * 110) in two places, so fixing this problem will require changing both call sites. To avoid such duplication, it would be better to have a Noise(char c) constructor that chains to the Noise(int freq) constructor.

Since the frequencies are spaced at uniform intervals of 110 Hz, the higher frequencies will sound similar in pitch. From e to f is one octave; going from v to w is a half step. That's fine, if that's the effect you want.

Your sampling frequency, 16 * 1024 Hz, is sufficiently high to provide good sound quality for the tones you want to generate. However, it's still a magic number pulled from nowhere. I suggest that you stick to conventional values, such as 44100 Hz (CD audio quality) or 48000 Hz (DVD audio quality).

Continuity

When you hold down a key, you'll hear something that sounds like machine gun fire, at 10 rounds per second. It appears that your intention is to play the tone continuously (possibly simultaneously with other tones) until the key is released, in which case a smooth sinusoid would be preferable.

The main cause for the machine-gun effect is that you are writing sounds to the audio device without regard to what is already playing. A secondary problem is that you write 214 samples, which is in general not an integral number of wave periods.

The way to play a waveform repeatedly is not to have your own thread pump the audio data to the output, but to use Clip.loop(Clip.LOOP_CONTINUOUSLY).

Your KeyListener handlers need adjusting as well. I would use just keyPressed() and keyReleased(), not keyTyped(). In keyReleased(), the finally is superfluous.

Constructors

Constructors should just construct objects. I don't expect that instantiating a Noise also starts playing it, since new Thread() doesn't imply starting the thread. Similarly, instantiating a Boop shouldn't also cause the frame to be shown, since that's not how JFrames behave. Automatically calling .show() in the constructor violates the substitutability principle for subclasses.

As per the documentation, Swing code should run on the Event Dispatch, not the main thread.

Noise should be a static inner class. af should be a static field of Boop.

Suggested solution

import java.awt.TextArea;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.util.HashMap;
import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.SourceDataLine;
import javax.swing.JFrame;
import javax.swing.SwingUtilities;

public class Boop extends JFrame implements KeyListener {
    private static class Noise {
        private int freq;
        private Clip clip;
        private static Clip currentClip;

        public Noise(char c) {
            this((c % 20 + 1) * 110);
        }

        public Noise(int freq) {
            this.freq = freq;
            double slice = 2 * Math.PI * freq / SAMPLE_RATE;
            byte[] waveform = new byte[(int)(SAMPLE_RATE / freq + 1)];
            for (int i = 0; i  noises = new HashMap<>();

    private final TextArea textArea;

    public Boop() {
        textArea = new TextArea(20, 80);
        textArea.addKeyListener(this);
        add(textArea);
        pack();
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    public void playText(String sz) {
        for (char c: sz.toCharArray()) {
            (new Noise(c)).play(100);
        }
    }

    @Override
    public void keyTyped(KeyEvent ke) {
    }

    @Override
    public void keyPressed(KeyEvent ke) {
        if (ke.getKeyCode() == KeyEvent.VK_ENTER) {
            playText(textArea.getText().toString());
        } else {
            char ch = ke.getKeyChar();
            if (!noises.containsKey(ch)) {
                Noise t = new Noise(ch);
                noises.put(ch, t);
                t.start();
            }
        }
    }

    @Override
    public void keyReleased(KeyEvent ke) {
        Noise n = noises.get(ke.getKeyChar());
        if (n != null) {
            n.stop();
            noises.remove(ke.getKeyChar());
        }
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                (new Boop()).setVisible(true);
            }
        });
    }
}

Code Snippets

import java.awt.TextArea;
import java.awt.event.KeyEvent;
import java.awt.event.KeyListener;
import java.util.HashMap;
import javax.sound.sampled.AudioFormat;
import javax.sound.sampled.AudioSystem;
import javax.sound.sampled.Clip;
import javax.sound.sampled.LineUnavailableException;
import javax.sound.sampled.SourceDataLine;
import javax.swing.JFrame;
import javax.swing.SwingUtilities;

public class Boop extends JFrame implements KeyListener {
    private static class Noise {
        private int freq;
        private Clip clip;
        private static Clip currentClip;

        public Noise(char c) {
            this((c % 20 + 1) * 110);
        }

        public Noise(int freq) {
            this.freq = freq;
            double slice = 2 * Math.PI * freq / SAMPLE_RATE;
            byte[] waveform = new byte[(int)(SAMPLE_RATE / freq + 1)];
            for (int i = 0; i < waveform.length; i++) {
                waveform[i] = (byte)(127 * Math.sin(i * slice));
            }

            try {
                this.clip = AudioSystem.getClip();
                this.clip.open(af, waveform, 0, waveform.length);
            } catch (LineUnavailableException ex) {
                ex.printStackTrace();
            }
        }

        public void play(int msec) {
            if (this.clip != null) {
                if (Noise.currentClip != null) {
                    Noise.currentClip.drain();
                }
                Noise.currentClip = this.clip;
                this.clip.loop(msec * this.freq / 1000);
            }
        }

        public void start() {
            if (this.clip != null) {
                this.clip.loop(Clip.LOOP_CONTINUOUSLY);
            }
        }

        public void stop() {
            if (this.clip != null) {
                this.clip.stop();
            }
        }
    }

    private static final int SAMPLE_RATE = 44100;
    private static final AudioFormat af = 
            new AudioFormat(SAMPLE_RATE, 8, 1, true, true);
    private HashMap<Character, Noise> noises = new HashMap<>();

    private final TextArea textArea;

    public Boop() {
        textArea = new TextArea(20, 80);
        textArea.addKeyListener(this);
        add(textArea);
        pack();
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    }

    public void playText(String sz) {
        for (char c: sz.toCharArray()) {
            (new Noise(c)).play(100);
        }
    }

    @Override
    public void keyTyped(KeyEvent ke) {
    }

    @Override
    public void keyPressed(KeyEvent ke) {
        if (ke.getKeyCode() == KeyEvent.VK_ENTER) {
            playText(textArea.getText().toString());
        } else {
            char ch = ke.getKeyChar();
            if (!noises.containsKey(ch)) {
                Noise t = new Noise(ch);
                noises.put(ch, t);
                t.start();
            }
        }
    }

    @Override
    public void keyReleased(KeyEvent ke) {
        Noise n = noises.get(ke.getKeyChar());
        if (n !

Context

StackExchange Code Review Q#92964, answer score: 7

Revisions (0)

No revisions yet.