0

I have extended my codes with having an additional insertion sort algorithm to be chosen by giving the boolean value insertionSort either true or false when constructing a class instance. But when I execute my codes, then i get Stackoverflow error. The codes are as follows:

    import java.util.Random;

    /**
     * Write a description of class QuickSort1 here.
     * 
     * @author (your name) 
     * @version (a version number or a date)
     */
    public class QuickSort1 implements IntSorter
    {
        private int[] v;
        private Random randomGenerator;
        private boolean insertionSort;
        private InsertionSort insertionSorter;

        public QuickSort1(boolean useInsertionSort)
        {
            randomGenerator = new Random();
            insertionSort = useInsertionSort;
            if(insertionSort)
                insertionSorter = new InsertionSort();
        }

        public void sort(int[] v)
        {
            this.v = v;
            if(this.v.length > 0) {
                quickSort(this.v, 0, this.v.length-1);
            }
            else {
                return;
            }
        }

        private void quickSort(int[] v, int first, int last)
        {
            final int startInsertion = 20;
            int First = first;
            int Last = last;
            int pivot = v[randomGenerator.nextInt(v.length)];        

            if(Last-First<2 && !insertionSort)
                return;
            else if(insertionSort) {
                if(pivot >= Last-startInsertion)
                    v = insertionSorter.sort(v);
            }
            else {
                while(First <= Last) {
                    while(v[First] < pivot) {
                        First++;
                    }
                    while(v[Last] > pivot) {
                        Last--;
                        if(Last==0)
                            break;
                    }
                    if(First<=Last) {
                        int temp = v[First];
                        v[First] = v[Last];
                        v[Last] = temp;
                        First++;
                        Last--;
                    }
                }

                if(first < Last

)
                quickSort(v, first, Last);
            if(First < last)
                quickSort(v, First, last);
        }        
    }

    public boolean getInfo()
    {
        return insertionSort;
    }
}

For the alternative insertion algorithm I have implemented a simple class with the following codes:

/**
 * Write a description of class InsertionSort here.
 * 
 * @author (your name) 
 * @version (a version number or a date)
 */
public class InsertionSort
{
    int[] v;

    /**
     * Constructor for objects of class InsertionSort
     */
    public InsertionSort()
    {
    }

    public int[] sort(int[] v)
    {
        for(int i=1;i<v.length;i++) {
            int temp = v[i];
            int j;
            for(j=i-1;j>=0 && temp<v[j];j--) {
                v[j+1] = v[j];
            }
            v[j+1] = temp;
        }
        return v;
    }
}

The error messages I now get for executing this algorithm for arrays with the size of 10.000-100.000 elements are the following:

java.lang.StackOverflowError
    at java.util.Random.nextInt(Random.java:307)
    at QuickSort1.quickSort(QuickSort1.java:40)
    at QuickSort1.quickSort(QuickSort1.java:68)

The error at line 68 gets reapeted in the terminal a lot of times and it indicates on the first recursive call of the quickSort method. The line 40 indicates on the line where the pivot element gets decided by Java's randomizing int generator.

I have a strong feeling that this algorithm perhaps cannot be better than it is now since for bigger number of elements, the stack will get empty during the execution for great numbers of elements to be sorted and therefore I perhaps get that StackOverflowError. But perhaps you have another opinion about this problem?

Thanks in advance for helping me out with this :D

  • I strongly recommend you step through this with a debugger. You'll be able to see immediately what's going on. – Dawood ibn Kareem Feb 26 '15 at 18:05
  • 1
    `first`, `First`, `last` and `Last`!? – timrau Feb 26 '15 at 18:06
  • @timrau: you're right, it should be more better choices of name on the variables :D – user2566415 Feb 26 '15 at 18:09
  • @David Wallace: I had, but it just keeps going and going, I can't advance further than just seeing repetition of the method call. And I mean that this algorithm is supposed to be able to sort maximally 100.000 elements, then the debugger will not fit that good anylonger for searching the problem. For few elements, then the implementation works well. – user2566415 Feb 26 '15 at 18:12
  • Would it be helpful for you if I post you a working quick sort? – Andy_Lima Feb 26 '15 at 18:59
  • @Andy_Lima, is there any problems with my implementation? Is it hopeless to make my codes better? – user2566415 Feb 26 '15 at 19:04
  • Suggestion 1: change `First` and `Last` to `low` and `high` or something like that. It'll help you visualize things better. Suggestion 2, each time you're about to recurse, print out the values you're about to pass in. Suggestion 3: Post a sample `v` (if you can do so with a reasonable number of elements) that breaks it so we can actually see the breakage in action. – dcsohl Feb 26 '15 at 19:09
  • Let me see what I can do for you. – Andy_Lima Feb 26 '15 at 19:11
  • Oh, and the index of your `pivot` should be between your `first` and `last` elements, not chosen randomly from the array as a whole. – dcsohl Feb 26 '15 at 19:12
  • 1
    Your quicksort is working fine without the InsertionSort. I think you should [just increase your heap](http://stackoverflow.com/questions/15313393/how-to-increase-heap-size-in-eclipse) size. For what is the InsertionSort? – Andy_Lima Feb 26 '15 at 19:31
  • The stack overflow comes from recursion. Big arrays cause deep recursion, which produces stack overflow. Either increase your heap or implement an iterative quicksort. – CoronA Feb 26 '15 at 19:37
  • @Andy_Lima, the insertion sort is just an option that can be used after own preferences, it's relevant for a school project I am working on. – user2566415 Feb 26 '15 at 19:42
  • Which IDE you use and how much RAM does your PC provide? – Andy_Lima Feb 26 '15 at 19:47
  • @Andy_Lima, the IDE is Bluej and the RAM memory capacity is 4GB. How come? – user2566415 Feb 26 '15 at 19:54
  • @dcsohl, about the pivot element: could it be something like this: int pivot = v[randomGenerator.nextInt(Last-First)+Last]; ? – user2566415 Feb 26 '15 at 19:59
  • Sorry, meant: int pivot = v[randomGenerator.nextInt(Last-First)+First)]; – user2566415 Feb 26 '15 at 20:00
  • I might have solved the problem. The problem was that the index number for the pivot element was poorly coded. I have changed first to: `int pivot = v[randomGenerator.nextInt(Last-First)+First;` I have then got an error message that said I gave wrong argument to random generator (the argument was supposed to be positive), so then I changed it to as following: `int pivot = v[randomGenerator(Last-First+1)+First;` And suddenly, I didn't then got any stackoverflows. But is right to do so? My test codes does not object to this. – user2566415 Feb 26 '15 at 20:44
  • The specification of quick sort says that it is not a matter where you set the pivot point. But it also says it should be in the middle of the array. But i can not imagine that that was the reason. – Andy_Lima Feb 26 '15 at 20:52
  • Well, it is just that it is required in my school to have a pivot element whose index is randomly chosen between the first element and the last element. – user2566415 Feb 26 '15 at 21:00

1 Answers1

0

So here we go. I tried your code with a array size of 1,000,000, there 2GB RAM where not enough. So finally the answer will not be satisfying for you, but to point that problem out could be a nightlong program. But I think Stefan Mandel striked the nerve, so if you are interested, feel free to investigate in this direction. But now I won't let you go like that. I did your homework. This is working even with the regular heap size. Here is a example how to increase the heap. You'll only need the -Xmx1024 to increase the RAM to 1 GB.

public class QuickFooBar {

public static void main(String[] args) {
    Random random = new Random();
    int[] arr = new int[1000000];
    for (int i = 0; i < arr.length; ++i) {
        arr[i] = random.nextInt(1000000);
    }

    QuickSort1 test;
    test = new QuickSort1(false);
    test.setV(arr);
    test.quickSort(0, arr.length - 1);
    arr = test.getV();

    for (int i = 0; i < arr.length; ++i) {
        System.out.println(arr[i] + ", ");
    }
}

}

public class QuickSort1 {

private int[] arr;
private final boolean insertionSort;


public QuickSort1(final boolean insertionSort) {
    this.insertionSort = insertionSort;
}

public void quickSort(int left, int right) {
    final int startInsertion = 20;
    if (insertionSort) {
        if (((left + right) / 2) >= right - startInsertion) {
            arr = new InsertionSort().sort(arr);
            return;
        }
    }
    int index = partition(left, right);
    if (left < index - 1) {
        quickSort(left, index - 1);
    }
    if (index < right) {
        quickSort(index, right);
    }
}

private int partition(int left, int right) {
    int i = left, j = right;
    int tmp;
    int pivot = arr[new Random(right).nextInt()];
    while (i <= j) {
        while (arr[i] < pivot) {
            i++;
        }
        while (arr[j] > pivot) {
            j--;
        }
        if (i <= j) {
            tmp = arr[i];
            arr[i] = arr[j];
            arr[j] = tmp;
            i++;
            j--;
        }
    };
    return i;
}

public boolean getInfo() {
    return insertionSort;
}

public int[] getV() {
    return arr;
}

public void setV(int[] v) {
    this.arr = v;
}

}

public class InsertionSort {

public int[] sort(int[] v) {
    for (int i = 1; i < v.length; ++i) {
        int temp = v[i];
        int j;
        for (j = i - 1; j >= 0 && temp < v[j]; j--) {
            v[j + 1] = v[j];
        }
        v[j + 1] = temp;
    }
    return v;
}

}

    public int[] sort(int[] v) {
    for (int i = 1; i < v.length; ++i) {
        int temp = v[i];
        int j;
        for (j = i - 1; j >= 0 && temp < v[j]; j--) {
            v[j + 1] = v[j];
        }
        v[j + 1] = temp;
    }
    return v;
}

}

Community
  • 1
  • 1
Andy_Lima
  • 129
  • 12
  • I really appreciate the work you have done and for your time effort, but your solution does not contain a pivot element that is randomly chosen from the first point to last in an array. And I don't know, I might have solved on my hand by changing `int pivot = v[randomGenerator.nextInt(Last-First+1)+First];` and since then, I haven't got any errors with my program nor stackoverflows. But do you think it is enough? – user2566415 Feb 26 '15 at 20:59
  • Do you know know for what the seed in the random is? Anyways now you get a random pivot. – Andy_Lima Feb 26 '15 at 21:09
  • Well, you could if you have time develop the question about the random seed. I am not so put in when it comes to the random class structure etc. But isn't that the point, with having a pivot element that is randomly? – user2566415 Feb 26 '15 at 21:15
  • And sorry it's not said that it needs to increase calculating time but it could. If you want to give it a shot what is faster, the random or the center version let me know the result! – Andy_Lima Feb 26 '15 at 22:14