Sorting Algorithm Visualizer

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 list;
private ArrayList 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();
colorList = new ArrayList();
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(linevScale), this.getWidth(), y+Math.round(linevScale));
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, 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 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, in

StackExchange Code Review Q#75314, answer score: 4