Simple Java MIDI player

I created a simple program that allows you to create and play MIDI sounds.
I've used the MVC approach and I’d like to know if there are any improvements to be made regarding the design and structure of the classes? Have I used a constants class correctly? I also feel that the checkBoxArrayin the Midi class should be in a separate class. Should it be?

Please also let me know of any improvements to be made on the readability and general layout of the program.

For those interested, I've updated my code with the advice given on this thread: Simple Java MIDI player followup

BeatBox (main class)

package BeatBox;

public class BeatBox {
    GUI gui;
    Midi midi;

    public BeatBox(){
        midi = new Midi();
        gui = new GUI(midi);
        midi.setUpMidi();
    }

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

GUI class

```
package BeatBox;

import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.*;

public class GUI {
private Midi midi;

public GUI(Midi midi){
this.midi = midi;
buildGUI();
}

// buildGUI - creates the GUI for the beat box program
void buildGUI(){
JFrame frame = new JFrame("Cyber BeatBox");
frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);
frame.setMinimumSize(new Dimension(600, 350));
JPanel panel = new JPanel(new BorderLayout());
panel.setBorder(BorderFactory.createEmptyBorder(10, 10, 10, 10));
frame.setContentPane(panel);

// Instrument names
Box nameBox = new Box(BoxLayout.Y_AXIS);
for(int i = 0; i < BeatBoxConstants.NUM_INSTRUMENTS; i++){
nameBox.add(new Label(BeatBoxConstants.instrumentNames[i]));
}

// Check box
GridLayout grid = new GridLayout(16, 16);
grid.setVgap(0);
grid.setHgap(2);
JPanel checkBoxPanel = new JPanel(grid);
for(int i = 0; i < BeatBoxC

Disclaimer: I don't really know that much about music, so some of my assumptions might be wrong.


Have I used a constants class correctly?

I wouldn't say so. I would only use a constants class for things which are not part of the state of any objects, and which do not expose the inner workings of objects (eg a debug field, database credentials, fixed strings for user output, etc).

The numbers in instruments (which should be all upper case) seem to be random numbers (as in, they are not numbers a musician would recognize, but numbers that are specific to your program). They also seem to interact with instrumentNames, which is not good (see below).

NUM_INSTRUMENTS isn't really needed at all. NUM_BEATS is the only value that I might leave in a constants class.


I’d like to know if there are any improvements to be made regarding the design and structure of the classes?

One thing that directly jumped out at me is that it's really hard to see how you make the sounds of different instruments. It seems to depend on the passed key in makeBeat(int key, int tick). The call of that depends on the order of instruments in the instruments constant, and that it is in the correct order regarding instrumentNames, and displayed in the same order inside the GUI.

It's never a good idea to put that much weight on the order of different lists being correct (because those lists work independently of each other).

I would introduce an explicit Instrument class, which has a name and a key (if key is the technical term).

I would then pack the whole tick thing into a class as well (because of my lack of music knowledge it's hard to say how. Some ideas: Could a list of ticks be part of an instrument? Or could there be TickLists which each own one instrument?).


I also feel that the checkBoxArray in the Midi class should be in a separate class. Should it be?

Yes. Your Midi class is basically your model and controller, but it definitely isn't your GUI, so it shouldn't have JCheckBoxs. Instead, it should have the Instrument/TickList structure described above, which it can then expose to the GUI.

Misc

• a reset button would be great for usability, and should be easy to add.

• don't import *, instead make all your imports explicit, so readers know what classes you use.

• use JavaDoc style method comments (they are easier to read as they make input parameters and output explicit, and they can be displayed in IDE tooltips, published on the web, etc).

• Naming: seq can be a bit confusing, as it's unclear if it is sequencer or sequence; I would just write it out. At first, I thought comd is some music term, but as it stands for command, just write it out as well. Same for chan (channel).

StackExchange Code Review Q#83452, answer score: 3