3
\$\begingroup\$

Source

I made a project for a class, a MIDI editor written with Java 8 and Swing, and continued working on it on my own. However, I ran into a problem I couldn't solve and gave up.

I'd like feedback on high level design/architecture, things like cohesion, coupling etc between classes. I'm pretty sure the project is weak in this regard. Would MVC be appropriate here?

I'm also concerned about the model. I made it so that a composition has a list of measures, and a measure has a list of notes, but I think that was hard to work with and caused issues. Maybe a map data structure would have been better. The class for reading MIDI files became really complicated because I didn't model sequences and tracks.

The problem I couldn't solve was synchronizing the movement of the progress line and the audio playback. I know the focus of this site isn't to debug, so you don't have to comment on that.

Please feel free to offer any feedback at all also.

package ui;

import model.Composition;
import persistence.Reader;
import ui.players.EntirePlayer;
import ui.sound.MidiSynth;
import ui.tools.*;

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.Observable;
import java.util.Observer;

import static ui.CompositionPanel.SEMITONE_HEIGHT;

public class GraphicalEditorApp extends JFrame {
    public static final int WIDTH = 1500;
    public static final int HEIGHT = SEMITONE_HEIGHT * 88;
    //public static final int HEIGHT = SEMITONE_HEIGHT * 70;
    //public static final String SAVE_FILE = "./data/saveFile.mid";
    public static final String SAVE_FILE = "./data/Gurenge.mid";

    private MidiSynth midiSynth;
    private EntirePlayer player;
    private Timer masterTimer;

    private CompositionPanel compositionPanel;
    private JScrollPane scroller;

    private List<Tool> tools; // is this even useful??
    private Tool activeTool;

    public GraphicalEditorApp() {
        super("Music Editor");
        initFields();
        initGraphics();
        initSound();
        showInitDialog();
        initInteraction();
    }

    public MidiSynth getMidiSynth() {
        return midiSynth;
    }

    public CompositionPanel getCompositionPanel() {
        return compositionPanel;
    }

    private void initFields() {
        activeTool = null;
        compositionPanel = new CompositionPanel(1, 4, 4, this);
        //composition.addMeasures(1, 1, 4, 4);
        tools = new ArrayList<Tool>();
    }

    // EFFECTS: allows the user to input init options, such as to load from file or create new.
    private void showInitDialog() {
        int closedOption = -1;
        int yes = 0;
        int no = 1;
        String[] options = {"Yes", "No"};
        int n = JOptionPane.showOptionDialog(this,
                "Would you like to load the previously saved file?",
                "Initialization",
                JOptionPane.YES_NO_CANCEL_OPTION,
                JOptionPane.QUESTION_MESSAGE,
                null,
                options,
                options[1]);
        if (n == closedOption) {
            return;
        } else if (n == yes) {
            loadProject();
        } else {
            return;
        }
    }

    // MODIFIES: this
    // EFFECTS: changes composition to the one contained in the save file
    private void loadProject() {

        compositionPanel.setComposition(Reader.readFile(new File(SAVE_FILE)));
        compositionPanel.getComposition().addMidiSynthToAll(midiSynth);
        // important!!
        player.setComposition(compositionPanel.getComposition());

        repaint();
    }


    private void initSound() {
        midiSynth = new MidiSynth();
        midiSynth.open();
    }

    private void initGraphics() {
        setLayout(new BorderLayout());
        setMinimumSize(new Dimension(WIDTH, HEIGHT));
        createTools();
        createNavigationBar();
        addComposition();
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setLocationRelativeTo(null);
        setVisible(true);
        // necessary if size of composition may change???
        setBackground(Color.black);
    }

    private void createNavigationBar() {
        //JToolBar navigationBar = new JToolBar("Navigation");
        JButton button = new JButton("Jump to");
        JTextField measureNumber = new JTextField();
        NavigationBar bar = new NavigationBar("Navigation", button, measureNumber, this);
        add(bar, BorderLayout.PAGE_START);


    }

    // MODIFIES: this
    // EFFECTS: adds a composition component to the editor
    private void addComposition() {
        //add(compositionPanel, BorderLayout.CENTER);
        compositionPanel.setPreferredSize(new Dimension(WIDTH, HEIGHT));
        scroller = new JScrollPane(compositionPanel);
        add(scroller, BorderLayout.CENTER);
        validate();
    }

    public JScrollPane getScroller() {
        return scroller;
    }

    // MODIFIES: this
    // EFFECTS: declare and instantiate all tools
    private void createTools() {
        JPanel toolbar = new JPanel();
        toolbar.setLayout(new GridLayout(0, 1));
        toolbar.setSize((new Dimension(0, 0)));
        add(toolbar, BorderLayout.EAST);

        AddNoteTool addNoteTool = new AddNoteTool(this, toolbar);
        tools.add(addNoteTool);
// should I add the others to the list?? What is the point of the list?
        AddMeasuresTool addMeasuresTool = new AddMeasuresTool(this, toolbar);

        EditNoteTool editNoteTool = new EditNoteTool(this, toolbar);

        RemoveMeasuresTool removeMeasuresTool = new RemoveMeasuresTool(this, toolbar);

        masterTimer = new Timer(0, null);
        player = new EntirePlayer(compositionPanel.getComposition(), null, null);
        PlayEntireTool playEntireTool = new PlayEntireTool(this, toolbar, player);
        player.setTool(playEntireTool);
        //player.setCompositionPanel(compositionPanel);


        SaveTool saveTool = new SaveTool(this, toolbar);

        setActiveTool(addNoteTool);
    }

    public EntirePlayer getPlayer() {
        return player;
    }

    public Timer getMasterTimer() {
        return masterTimer;
    }

    public void setMasterTimer(Timer masterTimer) {
        this.masterTimer = masterTimer;
    }

    // MODIFIES: this
    // EFFECTS: initializes EditorMouseListener and EKL for JFrame
    private void initInteraction() {
        EditorMouseListener eml = new EditorMouseListener();
        compositionPanel.addMouseListener(eml);
        compositionPanel.addMouseMotionListener(eml);

        //EditorKeyListener ekl = new EditorKeyListener();
        //addKeyListener(ekl);
        //compositionPanel.addKeyListener(ekl);
        //compositionPanel.setFocusable(true);
        //compositionPanel.requestFocusInWindow();


    }

    // MODIFIES: this
    // EFFECTS:  sets the given tool as the activeTool
    public void setActiveTool(Tool tool) {
        if (activeTool != null) {
            activeTool.deactivate();
        }
        tool.activate();
        activeTool = tool;
    }

    // EFFECTS: if activeTool != null, then mousePressedInDrawingArea is invoked on activeTool, depends on the
    //          type of the tool which is currently activeTool
    private void handleMousePressed(MouseEvent e) {
        if (activeTool != null) {
            activeTool.mousePressed(e);
        }
        repaint();
    }

    // EFFECTS: if activeTool != null, then mouseReleasedInDrawingArea is invoked on activeTool, depends on the
    //          type of the tool which is currently activeTool
    private void handleMouseReleased(MouseEvent e) {
        if (activeTool != null) {
            activeTool.mouseReleased(e);
        }
        repaint();
    }

    // EFFECTS: if activeTool != null, then mouseClickedInDrawingArea is invoked on activeTool, depends on the
    //          type of the tool which is currently activeTool
    private void handleMouseClicked(MouseEvent e) {
        if (activeTool != null) {
            activeTool.mouseClicked(e);
        }
        repaint();
    }

    // EFFECTS: if activeTool != null, then mouseDraggedInDrawingArea is invoked on activeTool, depends on the
    //          type of the tool which is currently activeTool
    private void handleMouseDragged(MouseEvent e) {
        if (activeTool != null) {
            activeTool.mouseDragged(e);
        }
        repaint();
    }

    // EFFECTS: if activeTool != null, then keytyped is invoked on activeTool.
    private void handleKeyTyped(KeyEvent ke) {
        if (activeTool != null) {
            activeTool.keyTyped(ke);
        }
        repaint();
    }



    // from SimpleDrawingPlayer (modified)
    private class EditorMouseListener extends MouseAdapter {
        // EFFECTS: Forward mouse pressed event to the active tool
        public void mousePressed(MouseEvent e) {
            handleMousePressed(translateEvent(e));
        }

        // EFFECTS: Forward mouse released event to the active tool
        public void mouseReleased(MouseEvent e) {
            handleMouseReleased(translateEvent(e));
        }

        // EFFECTS:Forward mouse clicked event to the active tool
        public void mouseClicked(MouseEvent e) {
            handleMouseClicked(translateEvent(e));
        }

        // EFFECTS:Forward mouse dragged event to the active tool
        public void mouseDragged(MouseEvent e) {
            handleMouseDragged(translateEvent(e));
        }

        // EFFECTS: translates the mouse event to current drawing's coordinate system
        private MouseEvent translateEvent(MouseEvent e) {
            return SwingUtilities.convertMouseEvent(e.getComponent(), e, compositionPanel);
        }
    }



}
package ui;

import model.Composition;
import model.Measure;

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

// a beat refers to a quarter beat.
public class CompositionPanel extends JPanel implements ActionListener, Scrollable {
    private int playLineColumn;
    private GraphicalEditorApp editor;
    private Composition composition;

    public static final int DEFAULT_BPM = 80;

    public static final int MAXIMUM_BEAT_WIDTH = 512;
    public static final int MINIMUM_BEAT_WIDTH = 1;
    public static final int ZOOM_INTERVAL = 10;

    public static int bpm;
    //public static int resolution = 4; // initial tick is a 16th note
    public static int tickWidth = 16;
    //public static int beatWidth = tickWidth * resolution; //50, then tried 64
    public static int beatWidth = 64;
    public static final int SEMITONE_HEIGHT = 10;


    public CompositionPanel(int numMeasures, int beatNum, int beatType, GraphicalEditorApp editor) {
        super();
        setBackground(Color.black);
        composition = new Composition(numMeasures, beatNum, beatType);
        this.editor = editor;
        bpm = DEFAULT_BPM;
    }

    @Override
    // MODIFIES: this
    // EFFECTS: update this every time the timer fires by incrementing playLineColumn by one screen coordinate
    // and scroll following.
    public void actionPerformed(ActionEvent e) {
        playLineColumn += 1;
        repaint();
        followPlaying();
        //if (editor.getMasterTimer().getDelay() == )
    }

    // MODIFIES: this
    // EFFECTS: stops the timer and resets the playLineColumn to 0.
    public void stopPlaying() {
        editor.getMasterTimer().stop();
        playLineColumn = 0;
        repaint();
    }

    // MODIFIES: this
    // EFFECTS: sets the value of beatWidth and makes sure invariant relationships between static variables
    // are maintained.
    // value writes even of public static variables should be done locally and in one place so the repercussions can be
    // handled in one place.
    public void setBeatWidth(int newBeatWidth) {
        float factor = (float) newBeatWidth / (float) beatWidth;
        beatWidth = newBeatWidth;
        tickWidth = Math.round(factor * tickWidth);
    }

    // EFFECTS: gets the end of the composition panel in screen x coordinate.
    // OUTDATED
    public int getEnd() {
        int totalTicks = composition.getNumTicks();
        return totalTicks * tickWidth;
        //int totalBeats = composition.getNumBeats();
        //return totalBeats * beatWidth;
    }

    // MODIFIES: this
    // EFFECTS: changes the dimensions of this. If the new width is greater than the starting width, assign
    // the new width. Otherwise do nothing.
    public void resize() {
        if (getEnd() > editor.WIDTH) {
            setPreferredSize(new Dimension(getEnd(), editor.HEIGHT));
        }
        revalidate();
    }

    public void setBPM(int newBPM) {
        bpm = newBPM;
    }

    public Composition getComposition() {
        return composition;
    }


    // EFFECTS: sets composition to the given one, and ensures that the ratio of tickwidth to beatwidth
    // obeys the resolution of the composition.
    public void setComposition(Composition composition) {
        this.composition = composition;
        int amount = beatWidth / Composition.resolution;
        if (amount == 0) {
            amount = 1;
        }
        tickWidth = amount;
        resize();
    }




    // EFFECTS: paints grid, playing line, notes in composition.
    // calls to repaint() get here
    @Override
    public void paintComponent(Graphics graphics) {
        super.paintComponent(graphics);
        drawLines(graphics);
        // it may be better for measure.draw to draw bar lines.
        for (Measure measure : composition.getListOfMeasure()) {
            measure.draw(graphics);
        }
    }

    // EFFECTS: draws semitone lines and bar lines
    private void drawLines(Graphics graphics) {
        int endX = 0;
        Color save = graphics.getColor();
        graphics.setColor(new Color(100, 100, 200));
        // 100 100 200

        List<Integer> positions = composition.getPositionsOfMeasureLines();
        for (Integer x : positions) {
            graphics.drawLine(x, 0, x, getHeight());
            endX = x;
        }

        for (int y = SEMITONE_HEIGHT; y < getHeight(); y += SEMITONE_HEIGHT) {
            graphics.drawLine(0, y, endX, y);
        }

        if (playLineColumn > 0 && playLineColumn < getWidth()) {
            graphics.setColor(Color.RED);
            graphics.drawLine(playLineColumn, 0, playLineColumn, getHeight());
        }
        graphics.setColor(save);
    }

    // MODIFIES: this
    // EFFECTS: sets value of playLineColumn
    public void setPlayLineColumn(int target) {
        playLineColumn = target;
    }

    // MODIFIES: editor
    // EFFECTS: makes the editor's scroller keep the playing line in the middle of the view.
    public void followPlaying() {
        int leftEndOfView = playLineColumn - (editor.WIDTH / 2);
        editor.getScroller().getHorizontalScrollBar().setValue(leftEndOfView);
    }
}
package model;

import ui.sound.MidiSynth;

import java.awt.*;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import static ui.CompositionPanel.*;
// consider splitting graphical responsibilities into new class

// represents an entire composition, which is a collection of measures. The order of measures in the list is the order
// of measures in the composition.
public class Composition  { // used to extend JPanel
    private List<Measure> listOfMeasure;
    //private int beatsPerMinute;
    private int beatNum;
    private int beatType;
    private int barWidth; // implement later
    public static int resolution = 4;
    // the resolution is the number of ticks per quarter beat.


    // REQUIRES: beatType is a power of 2
    // EFFECTS: instantiates a new composition with numMeasures measures, beatNum beats of type beatType per measure.
    public Composition(int numMeasures, int beatNum, int beatType) {
        //super();
        //setBackground(Color.black);
        this.beatNum = beatNum;
        this.beatType = beatType;
        listOfMeasure = new ArrayList<Measure>();
        for (int i = 0; i < numMeasures; i++) {
            Measure tempMeasure = new Measure(beatNum, beatType, this, i + 1);
            listOfMeasure.add(tempMeasure);
        }
    }

    public void setResolution(int resolution) {
        Composition.resolution = resolution;
    }

    // OBSOLETE
    // REQUIRES: beat > 0
    // EFFECTS: returns the measure which has the given global beat within it. If none have the given beat, return null.
    public Measure getMeasureAtBeat(int beat) {
        int beatCount = 0;
        for (Measure measure: listOfMeasure) {
            beatCount += measure.getBeatNumber();
            if (beatCount >= beat) {
                return measure;
            }
        }
        return null;
    }

    // REQUIRES: tick > 0
    // EFFECTS: returns the measure which has the given tick within it. If none, return null.
    public Measure getMeasureAtTick(int tick) {
        int tickCount = 0;
        for (Measure m: listOfMeasure) {
            tickCount += m.getNumTicks();
            if (tickCount >= tick) {
                return m;
            }
        }
        return null;
    }

    // EFFECTS: adds a midisynth to all notes.
    public void addMidiSynthToAll(MidiSynth midiSynth) {
        for (Measure measure : listOfMeasure) {
            measure.addMidiSynthToAll(midiSynth);
        }
    }

    // EFFECTS: returns array containing the x screen coordinates of all measure lines in the composition.
    public List<Integer> getPositionsOfMeasureLines() {
        List<Integer> output = new ArrayList<Integer>();
        output.add(0);
        int tickCount = 0;
        for (Measure m: listOfMeasure) {
            tickCount += m.getNumTicks();
            output.add(tickCount * tickWidth);
        }
        return output;
    }


    // EFFECTS: returns the global start tick of the given measure.
    public int getGlobalStartOf(Measure measure) {
        int output = 0;
        for (Measure m: listOfMeasure) {
            if (!m.equals(measure)) {
                output += m.getNumTicks();
            } else {
                output++;
                return output;
            }
        }
        return 0; //this should throw an exception probably
    }

    // EFFECTS: returns the note at a given point in composition, if any.
    public Note getNoteAtPoint(Point point) {
        for (Measure measure : listOfMeasure) {
            for (Note note : measure.getListOfNote()) {
                if (note.contains(point)) {
                    return note;
                }
            }
        }
        return null;
    }

    // EFFECTS: returns list of notes at a given column in the composition.
    public List<Note> getNotesAtColumn(int x) {
        List<Note> notesAtColumn = new ArrayList<Note>();
        for (Measure m: listOfMeasure) {
            for (Note note : m.getListOfNote()) {
                if (note.containsX(x)) {
                    notesAtColumn.add(note);
                }
            }
        }
        return notesAtColumn;
    }

    // EFFECTS: returns list of notes at a given tick in the composition.
    public List<Note> getNotesAtTick(int tick) {
        List<Note> notesAtTick = new ArrayList<>();
        /*
        for (Measure m: listOfMeasure) {
            for (Note note : m.getListOfNote()) {
                if (note.containsTick(tick)) {
                    notesAtTick.add(note);
                }
            }
        }
         */

        int ticksPerMeasure = listOfMeasure.get(0).getNumTicks();
        int index = tick / ticksPerMeasure;
        Measure measure = listOfMeasure.get(index);
        for (Note note : measure.getListOfNote()) {
            if (note.containsTick(tick)) {
                notesAtTick.add(note);
            }
        }
        return notesAtTick;

    }

    // REQUIRES: pos <= number of measures, beatType is a power of 2
    // MODIFIES: this
    // EFFECTS: adds n new measures to composition after the posth measure with beatNum beats of type beatType.
    public void addMeasures(int n, int pos, int beatNum, int beatType) {
        //int prevSize = listOfMeasure.size();
        for (int i = 0; i < n; i++) {
            Measure tempMeasure = new Measure(beatNum, beatType, this, pos + i + 1);
            listOfMeasure.add(pos + i, tempMeasure);
            countAndNotifyMeasures();
            assert checkMeasureNums();
            //.add adds the element at the index and shifts everything afterwards to the right by one.
        }
    }

    // EFFECTS: checks the class invariant that the measures have correct measure numbers for their order in
    // listOfMeasure
    private boolean checkMeasureNums() {
        int count = 0;
        for (Measure measure : listOfMeasure) {
            count++;
            if (count != measure.getMeasureNumber()) {
                return false;
            }
        }
        return true;
    }

    // REQUIRES: there is a measure at every position specified.
    // MODIFIES: this
    // EFFECTS: removes the specified measures from composition, and makes sure note positions and measure numbers
    // resulting are updated.
    public void removeMeasures(List<Integer> listOfPos) {
        // reverse list to remove last first
        Collections.sort(listOfPos); // have to sort first
        Collections.reverse(listOfPos);
        for (Integer pos : listOfPos) {
            int tickDiff = -1 * listOfMeasure.get(pos - 1).getNumTicks();
            updateNotePositions(tickDiff, pos); // pos - 1 + 1
            listOfMeasure.remove(pos - 1);
        }
        countAndNotifyMeasures();
    }

    // MODIFIES: this
    // EFFECTS: for all measures from the one after the one to be removed until the end, adjust note starts by
    // tickDiff.
    private void updateNotePositions(int tickDiff, int index) {
        for (int i = index; i < listOfMeasure.size(); i++) {
            listOfMeasure.get(i).adjustNotes(tickDiff);
        }
    }

    // EFFECTS: notifies all measures of their measure numbers.
    private void countAndNotifyMeasures() {
        int count = 0;
        for (Measure measure: listOfMeasure) {
            count++;
            measure.setMeasureNumber(count);
        }
    }

    // EFFECTS: returns the number of measures in the composition.
    public int getNumMeasures() {
        return listOfMeasure.size();
    }

    // EFFECTS: returns the total number of beats in the composition.
    public int getNumBeats() {
        int numBeats = 0;
        for (Measure measure : listOfMeasure) {
            numBeats = numBeats + measure.getBeatNumber();
        }
        return numBeats;
    }

    // EFFECTS: returns the total number of ticks in the composition.
    public int getNumTicks() {
        int numTicks = 0;
        for (Measure measure : listOfMeasure) {
            numTicks += measure.getNumTicks();
        }
        return numTicks;
    }

    // EFFECTS: returns the measure at pos
    public Measure getMeasure(int pos) {
        return listOfMeasure.get(pos - 1);
    }
/*
    public int getPosOfMeasure(Measure measure) {
        return 0;
    }
*/

    // EFFECTS: return formatted content of composition
    public String getContents() {
        String tempString = "";
        for (int i = 0; i < listOfMeasure.size(); i++) {
            int n = i + 1;
            tempString = tempString + "\n" + "Measure" + n + "\n" + listOfMeasure.get(i).getContents();
        }
        return tempString;
    }

    // REQUIRES: measure not null
    // MODIFIES: this
    // EFFECTS: add measure to end of composition
    public void addMeasure(Measure measure) {
        measure.assignToComposition(this);
        listOfMeasure.add(measure);
        measure.setMeasureNumber(listOfMeasure.size());
    }

    public int getBeatNum() {
        return beatNum;
    }

    public int getBeatType() {
        return beatType;
    }

    public List<Measure> getListOfMeasure() {
        return listOfMeasure;
    }


}
\$\endgroup\$
3
  • 2
    \$\begingroup\$ Maybe you missed the text on the right side when asking: "Your question must contain code that is already working correctly, and the relevant code sections must be embedded in the question." Questions must include the code to be reviewed. Links to code hosted on third-party sites are permissible, but the most relevant excerpts must be embedded in the question itself. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Aug 15 '20 at 0:56
  • 2
    \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ noted and fixed. \$\endgroup\$ – ShokoN Aug 15 '20 at 2:40
  • 2
    \$\begingroup\$ This is a well asked question (now that it includes the code), but you may not get everything you want out of a code review. The Software Engineering site provides design reviews and that may give you more of what you are looking for, but make sure to read their help center. first. \$\endgroup\$ – pacmaninbw Aug 15 '20 at 12:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.