Sorting Visualizations - WinForms

Description

I think many many people have seen this youtube video "15 Sorting Algorithms in 6 Minutes"

When I saw it, I decided: "I can do that better" and I tried to get things done with the tools at my disposal back then. Windows Forms and C#. I added a functionality to allow the pausing of sorting in any point of time. Therefore I reasoned, my algoritms need to move step by step, and Implemented accordingly.

Since then much has changed and yesterday I revisited my code from then, and it was gruesome.

Not just bad, but horrible. So i went ahead and refactored what I could, removed superfluous stuff, decoupled implementations and algorithms, renamed methods to be more C#-ish, and realized, I have become a java-programmer by heart.

My method names got camelCased, there's no properties, and my Interface wasn't prefixed with I at the start.

Well, that aside, I finally arrived at something, where I am facing difficulties reducing the coupling. Where I am unsure what the best practice is, so here goes.

Code

Windows Forms oh how I love it, here come the FormDesigner.cs and the Form.cs ;)


`namespace SortVisualizations
{
partial class MainWindow
{
///
/// Erforderliche Designervariable.
///
private System.ComponentModel.IContainer components = null;

///
/// Verwendete Ressourcen bereinigen.
///
/// True, wenn verwaltete Ressourcen gelöscht werden sollen; andernfalls False.
protected override void Dispose(bool disposing)
{
if (disposing && (components != null))
{
components.Dispose();
}
base.Dispose(disposing);
}

#region Vom Windows Form-Designer generierter Code

///
/// Erforderliche Methode für die Designerunterstützung.
/// Der Inhalt der Methode darf nicht mit dem Code-Editor geändert werden.
///
private void InitializeComponent()
{

Naming

Methodnames in C# should be PascalCase

Some examples from your sources

private void init()
private void getStepAlgorithm(Algorithm selected)
private void generateRandomArrayElements()
private void uiInitializePaintArea()

As I read clock I thought, why is he showing a clock for a algorithm. I needed to check the declaration hidden inside the designer file to see it is a timer.

You should just name it timer.

private void getStepAlgorithm(Algorithm selected)
{
    this.stepAlgorithm = algos.GetAlgoImplementationFor(selected, this, ref arrayToSort);
}

Reading the methodname getStepAlgorithm let a reader assume that he is getting an algorithm but what the code does is assigning an algorithm to a class field. So a better name would be AssignAlgorithm

Decoupling

Communication between parent, here MainWindow, and child, here an inplementation of the IAlgoImplementation interface, should be by calling methods and setting of properties. From child to parent it should be done by events. The implementation of the IAlgoImplementation should know nothing of its parent. Also the factory should know nothing of the MainWindow.

public void SwapIndexes(int i, int p, ref int[] arrayToSort)

This method of the MainWindow class is violating separations of concerns, because it is called from the BubbleSort implementation of the IAlgoImplementation interface.

Refactoring

First we should change the name of interface IAlgoImplementation to interface ISortAlgorithm to make the name more meaningful. The same we do to the void Step() method and rename it to void PerformSortStep(). We add a property to hold the array to be sorted and add events for performing a sortstep, swapping values and for finishing the sorting.

interface ISortAlgorithm
{
    int[] ArrayToSort { set; }

    void PerformSortStep();

    event EventHandler SortStepPerformed;
    event EventHandler SwapPerformed;
    event EventHandler SortFinished;
}

public class SortStepPerformedEventArgs : EventArgs
{
    public int CurrentIndex { get; private set; }
    public PointOfStep StepPoint { get; private set; }
    public SortStepPerformedEventArgs(int currentIndex, PointOfStep stepPoint)
    {
        CurrentIndex = currentIndex;
        StepPoint = stepPoint;
    }
}

public enum PointOfStep
{
    BeforeStep, ReAssignedAssumeSorted, CurrentStep
}

public class SwapPerformedEventArgs : EventArgs
{
    public int FirstIndex { get; private set; }
    public int SecondIndex { get; private set; }
    public int FirstValue { get; private set; }
    public int SecondValue { get; private set; }
    public int LengthOfArrayToSort { get; set; }

    public SwapPerformedEventArgs(int firstIndex, int secondIndex, 
        int firstValue, int secondValue, 
        int lengthOfArrayToSort)
    {
        FirstIndex = firstIndex;
        SecondIndex = secondIndex;
        FirstValue = firstValue;
        SecondValue = secondValue;
        LengthOfArrayToSort = lengthOfArrayToSort;
    }

}

Now let us take a look at the BubbleSort class. As you didn't implement the interface explicit, I added a private get to the ArrayToSort property.

```
public class BubbleSort : ISortAlgorithm
{
private int current;
private int assumeSortedFrom;
private bool hasSwapped;

private int[] arrayToSort = null;
public int[] ArrayToSort
{
set {
if (value == null)
{
throw new ArgumentNullException("ArrayToSort", "The array to be sorted can't be null");
}
arrayToSort = value;
InitializeSorting();

}
private get { return arrayToSort; }
}

private void InitializeSorting()
{
current = 0;
assumeSortedFrom = ArrayToSort.Length;
hasSwapped = false;

}

public void PerformSortStep()
{

OnSortStepPerformed(current, PointOfStep.BeforeStep);

if (current + 1 == assumeSortedFrom)
{
assumeSortedFrom = current;
OnSortStepPerformed(current, PointOfStep.ReAssignedAssumeSorted);

current = 0;
OnSortStepPerformed(current, PointOfStep.CurrentStep);

if (!hasSwapped)
{
OnSortFinished();
}
else
{
hasSwapped = false;
}
}
else
{
if (ArrayToSort[current] > ArrayToSort[current + 1])
{
SwapIndexes(current, current + 1);
hasSwapped = true;
}

current++;
OnSortStepPerformed(current, PointOfStep.CurrentStep);
}
}

private void SwapIndexes(int currentIndex, int nextIndex)
{
int temp;

temp = arrayToSort[nextIndex];
ArrayToSort[nextIndex] = ArrayToSort[currentIndex];
ArrayToSort[currentIndex] = temp;

StackExchange Code Review Q#59625, answer score: 6