Sorting Algorithm Visualizer

Question

Over the course of the past month I've worked on a little visualizer to show how different algorithms are sorted. I'm pretty happy with how it turned out but would be interested in any feedback pertaining to the design of it or the coding.

I'm still a bit of a novice to Swing, so I'd love some feedback on my usage:

public class SortPanel extends JPanel {
private ArrayList<Integer> list;
private ArrayList<Colors> colorList;
private int hPad;
private int hRatio = 10; // ratio between width of bars and padding
private int width;
private int vPad = 5;
private int index;
private int line;
private float vScale;
private String name, message;

public SortPanel(String name) {
    super();
    list = new ArrayList<Integer>();
    colorList = new ArrayList<Colors>();
    this.name = name;
    message = "";
    index = 0;
    line = 0;
}

/*
...
Variety of setters and getters to manipulate the list of numbers
...
*/

public void paintComponent(Graphics g) {
    Graphics2D g2d = (Graphics2D) g;
    g2d.clearRect(0, 0, this.getWidth(), this.getHeight());
    g2d.drawString(name, 5, this.getHeight() - 30);
    g2d.drawString(message,5,this.getHeight() - 15);
    if (this.getListSize() > 0) {
        hPad = this.getWidth() / ((hRatio + 1) * list.size() + 1);
        vScale = (this.getHeight() - 2 * vPad - g2d.getFont().getSize() - 50)
                / (float) list.get(this.getMaxIndex());
        g2d.drawRect(0, 0, this.getWidth(), this.getHeight());
        width = hRatio * hPad;
        int y = vPad + 20;
        g2d.setColor(Colors.TARGET.get());
        g2d.drawLine(0, y+Math.round(line*vScale), this.getWidth(), y+Math.round(line*vScale));
        for (int i = 0; i < list.size(); i++) {
            int x = hPad * ((hRatio + 1) * i + 1);
            g2d.setColor(colorList.get(i).get());
            g2d.fillRect(x, y, width, Math.round(list.get(i) * vScale));
            if (i == index) { // index marker
                g2d.setColor(Color.RED);
                g2d.fillOval((2 * x + width) / 2 - 5, 5, 10, 10);
            }
        }
    }

}
}

This was also my first time using an enum. I wanted to make it easier to quickly get the Color I needed. Does this way provide less overhead and work more efficiently?

import java.awt.Color;

public enum Colors {
INACTIVE(191, 191, 191), SORTED(87, 232, 14), TARGET(255, 0, 0), ACTIVE(8,8,8), LOWER(12,94,245), UPPER(205,21,0);

private final Color col;

Colors(int r, int g, int b) {
    col = new Color(r, g, b);
}

public Color get() {
    return col;
}
}

I also used Threads for the first time here, so this is probably one of my main areas to look at.

public QuickSortThread(SortPanel sp, long msdelay) {
        super(sp, msdelay);
        sp.setIndex(-1);
    }
    public int partition(ArrayList<Integer> nums, int a, int b) {
        if (started) {
            int pivot = nums.get(b);
            sp.setLine(pivot);

            sp.setColorRange(a, b, Colors.ACTIVE);
            sp.setColor(b, Colors.TARGET);
            int greater = a;
            sp.setMessage("Moving elements before/after index " + greater
                    + " if they are < or > " + pivot + ".");
            for (int i = a; i < b && started; i++) {
                sp.setIndex(i);
                while (paused) {
                    try {
                        Thread.sleep(10);
                    } catch (Exception e) {
                        e.printStackTrace();
                    }
                }
                if (nums.get(i) < pivot) {
                    sp.setColor(i, Colors.LOWER);
                    sp.swap(i, greater);
                    greater++;
                    sp.setMessage("Moving elements before/after index "
                            + greater + " if they are < or > " + pivot
                            + ".");
                } else {
                    sp.setColor(i, Colors.UPPER);
                }
                sp.repaint();
                try {
                    Thread.sleep(msdelay);
                } catch (Exception ex) {
                    ex.printStackTrace();
                }
            }
            sp.swap(greater, b);
            sp.repaint();
            try {
                Thread.sleep(msdelay);
            } catch (Exception ex) {
                ex.printStackTrace();
            }

            return greater;
        }
        return -1;
    }

    public void quickSort(ArrayList<Integer> nums, int a, int b) {
        sp.setColorRange(0, Colors.INACTIVE);
        sp.repaint();
        if (a < b + 1 && started) {
            int pivot = partition(nums, a, b);
            quickSort(nums, a, pivot - 1);
            quickSort(nums, pivot + 1, b);
        }
    }

    public void run() {
        quickSort(list, 0, list.size() - 1);
        if (started) {
            sorted = true;
            sp.setLine(0);
            sp.setMessage("Sorted!");
            sp.setColorRange(0, Colors.SORTED);
            sp.setIndex(-1);
            sp.repaint();
        }

        if (checkAllSorted() && started) {
            MainWindow.this.start();
        }
    }
}

One thing I couldn't figure out was reducing the repeated Thread.sleep() blocks to methods. When I did so they stopped working. General OOP advice is also appreciated.

Here is a Gist of my code, and here is an executable jar.

Answer 1

First of all, nice effort.

Some of the things that I can say about is -

Visibility of the UI

There have been suggestions that for Swing we should use something like SwingUtilities.invokeLater() for making the UI visible.
So instead of using

public MainWindow() {
    ...
    frame.add(buttonPanel);
    frame.add(numbersPane);
    frame.setVisible(true);
}
...
public static void main(String[] args) {
    MainWindow mw = new MainWindow();
}

you can use something like -

public MainWindow() {
    ...
    frame.add(buttonPanel);
    frame.add(numbersPane);
}
public void startDisplay() {
    frame.setVisible(true);
}
...
public static void main(String[] args) {
    final MainWindow mw = new MainWindow();
    SwingUtilities.invokeLater(new Runnable() {
        @Override
        public void run() {
            mw.startDisplay();
        }
    });
}

Invocation of UI calls on a separate thread

• Whenever you're calling sp.repaint();, you're doing it on a separate thread. Shouldn't you do it on the Event Dispatcher Thread (EDT)?
• Also I couldn't get the purpose of Thread.sleep(msdelay); after every call to repaint. Could you maybe add some more description as to why you've done that?

Instead of sp.repaint(), do something like this -

SwingUtilities.invokeAndWait(new Runnable() {
    @Override
    public void run() {
        sp.repaint();
    }
});

and then add a breakpoint at sp.repaint() and even public void actionPerformed(ActionEvent event) { ... } to see how exactly the code is run on EDT.

---

Separation of Code and UI

• Everything is dumped in a single class MainWindow. Why not separate the
  implementations of SelectionSortThread, InsertionSortThread etc., after all
  you have already made separate classes for them.
• The entire implementation of Selection Sort has been done inside of run().
  Please separate them as you have done in case of Merge Sort (mergeSort() and
  merge()).
• Store the instance of SortPanel that you pass to the constructor.
  

So instead of -

class SelectionSortThread extends SortThread {
    public SelectionSortThread(SortPanel sp, long msdelay) {
        super(sp, msdelay);
    }
    ...
}

Have something like -

class SelectionSortThread extends SortThread {
    private SortPanel selectionSortPanel;
    public SelectionSortThread(SortPanel sp, long msdelay) {
        super(sp, msdelay);
        selectionSortPanel = sp;
    }
    ...
}

• Try to implement an MVC sort of thing.

Documentation and Nomenclature.

• Adding documentation would be nice.
  

  for e.g. I didn't know what the method paintComponent() did, later I realized that the method was initially declared in JComponent and you have overridden it. You could have just included the tag - @Override

• The variables - list and colorList are quite confusing. Give them a proper descriptive name like unsortedList etc.

---

Clean Code

I have moved some code around (I didn't have enough time to fork your repository, will do that later), so that it appears cleaner. Here is an example how you can abstract repeating code into new methods.

/** Merging */
public void merge(ArrayList<Integer> nums, int a, int mid, int b) {
    if (this.mainWindow.started) {
        int[] lower = new int[mid - a];
        int[] upper = new int[b - mid];

        int index = a;
        int i, j;
        for (i = 0; index < mid; i++, index++)
            lower[i] = nums.get(index);
        for (j = 0; index < b; j++, index++)
            upper[j] = nums.get(index);

        initialSP(a, mid, b);
        sleepThread(msdelay);

        i = 0; j = 0; index = a;
        while (i < lower.length && j < upper.length) {
            while (this.mainWindow.paused)
                sleepThread(10);

            if (lower[i] < upper[j]) {
                nums.set(index, lower[i]);
                changeSP(lower[i], index, Colors.LOWER);
                i++;
            } else {
                nums.set(index, upper[j]);
                changeSP(upper[j], index, Colors.UPPER);
                j++;
            }
            index++;
            sleepThread(msdelay);
        }

        while (i < lower.length) {
            while (this.mainWindow.paused)
                sleepThread(10);

            nums.set(index, lower[i]);
            changeSP(lower[i], index, Colors.LOWER);
            i++;
            index++;

            sleepThread(msdelay);
        }

        while (j < upper.length) {
            while (this.mainWindow.paused)
                sleepThread(10);

            nums.set(index, upper[j]);
            changeSP(upper[j], index, Colors.UPPER);
            j++;
            index++;

            sleepThread(msdelay);
        }
    }
}

private void sleepThread(long msdelay) {
    try {
        Thread.sleep(msdelay);
    } catch (Exception ex) {
        ex.printStackTrace();
    }
}

private void initialSP(int a, int mid, int b) {
    sp.setColorRange(0, Colors.INACTIVE);
    sp.setColorRange(a, mid, Colors.LOWER);
    sp.setColorRange(mid, b, Colors.UPPER);
    sp.setMessage("Merging values from index " + a + " to " + (b - 1)
            + " in order.");
    try {
        SwingUtilities.invokeAndWait(new Runnable() {
            @Override
            public void run() {
                sp.repaint();
            }
        });
    } catch (InvocationTargetException e) {
        e.printStackTrace();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}

private void changeSP(int line, int index, Colors color) {
    sp.setLine(line);
    sp.setColor(index, color);
    try {
        SwingUtilities.invokeAndWait(new Runnable() {
            @Override
            public void run() {
                sp.repaint();
            }
        });
    } catch (InvocationTargetException e) {
        e.printStackTrace();
    } catch (InterruptedException e) {
        e.printStackTrace();
    }
}