snippetcsharpModerate
Quick sort algorithm
Viewed 0 times
sortquickalgorithm
Problem
Is this algorithm good or can I improve it?
static void QuickSort(int[] a)
{
QuickSort(a, 0, a.Length - 1);
}
static void QuickSort(int[] a, int start, int end)
{
if (start >= end)
{
return;
}
int num = a[start];
int i = start, j = end;
while (i num)
{
j--;
}
a[i] = a[j];
while (i < j && a[i] < num)
{
i++;
}
a[j] = a[i];
}
a[i] = num;
QuickSort(a, start, i - 1);
QuickSort(a, i + 1, end);
}Solution
Though it is character-building to transform a recursive algorithm into an iterative form (and vice versa) I would not worry too much about iterative vs recursive.
There are several ways you could improve this code.
-
As I said in your previous post: why an array? You could be much more general and sort an
-
-
Recursive quicksort has four basic steps: (1) decide if we're already sorted, (2) choose a pivot, (3) partition and (4) recurse. The considerable majority of your algorithm is devoted to (3). Consider putting the partition logic in a helper method that can be clearly shown to be correct.
-
Consider showing us your test cases.
-
There is no error handling; what if the array is null? What if the start and end are out of bounds? And so on.
-
Consider adding postcondition assertions. A postcondition assertion is a
-
The partition step also has a postcondition; after the partition the array is partitioned into three parts: the values before the pivot are smaller or equal to it, the pivot, and the values after the pivot are greater than it. Assert that these conditions are met.
There are several ways you could improve this code.
-
As I said in your previous post: why an array? You could be much more general and sort an
IList. And why ints? You can sort any collection of things that can be compared consistently which would make your sorting algorithm more useful.-
start and end are very clear. i, j and num are not. How is the reader of this code supposed to understand what it does? Rename num to what it is: the pivot. -
Recursive quicksort has four basic steps: (1) decide if we're already sorted, (2) choose a pivot, (3) partition and (4) recurse. The considerable majority of your algorithm is devoted to (3). Consider putting the partition logic in a helper method that can be clearly shown to be correct.
-
Consider showing us your test cases.
-
There is no error handling; what if the array is null? What if the start and end are out of bounds? And so on.
-
Consider adding postcondition assertions. A postcondition assertion is a
Debug.Assert that documents what the method ensures is true just before it returns. In your case the postcondition is "the array is sorted from start to end". So assert that; write a little "is sorted" predicate and verify that it works. This will help you find bugs, if there are any. This will help future readers of the code understand it. And it will help people who change the code in the future understand what needs to not break when they modify the code.-
The partition step also has a postcondition; after the partition the array is partitioned into three parts: the values before the pivot are smaller or equal to it, the pivot, and the values after the pivot are greater than it. Assert that these conditions are met.
Context
StackExchange Code Review Q#142808, answer score: 11
Revisions (0)
No revisions yet.