HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Quicksort Algorithm Efficiency

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
algorithmefficiencyquicksort

Problem

I've just tried my hand at Quicksort, and I seem to have gotten it working (thanks to Stack Overflow). However, is it "true" Quicksort? And is it decently written?

public static void sort(List arr, int left, int right) {
        int i = left - 1;
        int j = right + 1;
        if (right - i == 0 || right - i == 1) return;
        int v = arr.get(right);     

        for(;;) {
            while(arr.get(++i) = j)break;

            Collections.swap(arr, i, j);
        }
        Collections.swap(arr, i, right);

        sort(arr, left, i - 1);
        sort(arr, i, right);
}


I know I should use a regular array (int[]) for speed, but we can ignore that for now.

Solution

int v = arr.get(right);


I assume v holds the partitioned item. But I don't know what you mean by v, so you can make your variable names more clear. Java's convention is to use CamelCase for variable names, so you might want to do that.

if (right - i == 0 || right - i == 1) return;


As I understand it, if right - i is either 0 or 1, the method will return. So why not just check if right - i is less than or equal 1?

if (right - i <= 1) return;


It might be more readable to put your statements inside braces, I think:

while(arr.get(++i) < v);
        while(v < arr.get(--j) && j != 0)


might be confusing: Perhaps it would be more understandable if you do this:

while(v < arr.get(--j) && j != 0) {
            if(j == 1)break;
        }


Regarding that code, the condition j != 0 will never be false because the loop will stop when j becomes 1. I think you can eliminate the if statement like this:

while(v  1);


Currently your method signature is:

public static void sort(List arr, int left, int right)


If we want to sort all items, we might call it as:

sort(list, 0, list.size()-1);


It might be better to write an overloaded method for convenience, Like this:

public static void sort(List arr) {
    sort(arr, 0, arr.size()-1);
}


Now if someone wants to sort all items in the list, they don't have to provide the beginning and end of the sort.

Code Snippets

int v = arr.get(right);
if (right - i == 0 || right - i == 1) return;
if (right - i <= 1) return;
while(arr.get(++i) < v);
        while(v < arr.get(--j) && j != 0)
while(v < arr.get(--j) && j != 0) {
            if(j == 1)break;
        }

Context

StackExchange Code Review Q#88243, answer score: 7

Revisions (0)

No revisions yet.