How can I implement mergesort for a ArrayList?

Question

I have a java code of mergesort for ArrayList but it doesn't sort correctly the ArrayList. But I don't find the mistake.

The code is:

public void mergeSort(ArrayList<Integer> list, int beg, int fin) {
    if (beg < fin) {
        int mid= (beg + fin) / 2;
        mergeSort(list, beg, mid);
        mergeSort(list, mid + 1, fin);
        Merge(list, beg, mid, fin);
    }
}


public void Merge(ArrayList<Integer> list, int beg, int mid, int fin) {
    ArrayList<Integer> left = new ArrayList<>(list.subList(beg, mid));
    ArrayList<Integer> right = new ArrayList<>(list.subList(mid, fin));
    
    int i = 0;
    int j = 0;
    int k = beg;
    
    while (i < left.size() && j < right.size()) {
        if (left.get(i) <= right.get(i)) {
            list.set(k, left.get(i));
            i++;
        } else {
            list.set(k, right.get(j));
            j++;
        }
        k++;
    }
    
    while (i < left.size()) {
        list.set(k, left.get(i));
        i++;
        k++;
    }

    while (j < right.size()) {
        try {
            list.set(k, right.get(j));
            j++;
            k++;
        }
    }
}

When I call it from other class...

jpanel4.mergeSort(jpanel4.lista, 0, jpanel4.lista.size() - 1);

(Merge is in jpanel4 class).

I converted a mergesort code for arrays to that, because it will work fine with other code that I have.

Thanks.

Answer 1

Check my version of merge sort using array list hope it help you    : https://github.com/murari99732/solutionleetcode-adventofcode/blob/master/PracticeAlgo/src/LeetCode/MergeSort.java

public static void mergeSort(List<Integer> arr, int start, int end) {
        if (start != end) {
            int mid = (start + end) / 2;
            mergeSort(arr, start, mid);
            mergeSort(arr, mid + 1, end);
            merge(arr, start, mid, end);
        }
    }

    private static void merge(List<Integer> arr, int start, int mid, int end) {
        int i = start;
        int j = mid + 1;
        int k = 0;

        int[] temp = new int[end - start + 1];

        while ((i <= mid) && (j <= end)) {
            if (arr.get(i) < arr.get(j))
                temp[k++] = arr.get(i++);
            else
                temp[k++] = arr.get(j++);
        }

        while (i <= mid) {
            temp[k++] = arr.get(i++);
        }

        while (j <= end) {
            temp[k++] = arr.get(j++);
        }

        for (i = start; i <= end; i++) {
            arr.remove(i);
            int e = temp[i - start];
            arr.add(i, e);
        }
    }

Answer 2

The main bug is a simple typo: if (left.get(i) <= right.get(i)) { should be:

    if (left.get(i) <= right.get(j)) {

You also have a problem with the array index values and boundaries: the beg and mid indices should be included and the fin (end?) should be excluded. This also removes the need for +1 / -1 adjustments.

There is a more subtile bug in int mid = (beg + fin) / 2;: computing this sum may overflow for very large arrays. It is better to write:

    int mid = beg + (fin - beg) / 2;

Once you correct these issues, you can remove the try block.

Here is a modified version:

public void mergeSort(ArrayList<Integer> list, int beg, int fin) {
    if (fin - beg >= 2) {
        int mid = beg + (fin - beg) / 2;
        mergeSort(list, beg, mid);
        mergeSort(list, mid, fin);
        Merge(list, beg, mid, fin);
    }
}

public void Merge(ArrayList<Integer> list, int beg, int mid, int fin) {
    ArrayList<Integer> left = new ArrayList<Integer>(list.subList(beg, mid));
    ArrayList<Integer> right = new ArrayList<Integer>(list.subList(mid, fin));
    
    int i = 0;
    int j = 0;
    int k = beg;
    
    while (i < left.size() && j < right.size()) {
        if (left.get(i) <= right.get(j)) {
            list.set(k, left.get(i++));
        } else {
            list.set(k, right.get(j++));
        }
        k++;
    }
    
    while (i < left.size()) {
        list.set(k, left.get(i++));
        k++;
    }

    while (j < right.size()) {
        list.set(k, right.get(j++));
        k++;
    }
}

You would call this method with jpanel4.mergeSort(jpanel4.lista, 0, jpanel4.lista.size());

Note that Merge can be simplified as the last loop is redundant, the remaining elements from right are already at the end of list. As a matter of fact, there is no need to save the elements from the right part as they are always copied to lower or equal index values.

Here is a simplified version:

public void Merge(ArrayList<Integer> list, int beg, int mid, int fin) {
    ArrayList<Integer> left = new ArrayList<Integer>(list.subList(beg, mid));
    
    for (int i = 0, j = mid, k = beg; i < left.size(); k++) {
        if (j >= fin || left.get(i) <= list.get(j)) {
            list.set(k, left.get(i++));
        } else {
            list.set(k, list.get(j++));
        }
    }
}

Answer 3

Let's use half-closed interval [beg, fin) because this approach is used in the most of in-build methods of Java, C++, etc.

Then call should be:

jpanel4.mergeSort(jpanel4.lista, 0, jpanel4.lista.size());
//                                                      ^ no -1

Now let's see how you divide given interval. Let's use [beg, mid) and [mid, end) intervals. If l >= r, then interval is empty.

But if given interval has length 1, it's going to be divided into two intervals of length 0 and 1 and mergeSort will be recursively called to sort the exact same interval. You don't need to sort one-element array, it's already sorted.

So mergeSort should look like:

    public void mergeSort(ArrayList<Integer> list, int beg, int fin){
        if(beg + 1 < fin){
        //     ^^^ added +1 to ensure we sort only intervals with length >= 2
            int mid = (beg+fin)/2;
            mergeSort(list, beg, mid);
            mergeSort(list, mid, fin);
            //                 ^ no +1
            Merge(list, beg, mid, fin);
        }
    }

Also there's a mistake in Merge:

    public void Merge(ArrayList<Integer> list, int beg, int mid, int fin){

        // ...

        while (i < left.size() && j < right.size()) {
            if (left.get(i) <= right.get(j)) {
            //                           ^ you should compare left[i] and right[j]
                list.set(k, left.get(i));
                i++;
                
            } else {
                list.set(k, right.get(j));
                j++;
            }
            
            k++;
        }

        // ...

     }