Java Recursive MergeSort for ArrayLists

Question

I have been having a problem with my mergesort function, as I am not able to sort a series of integers or strings whenever inputting it into the program. I have an outside class that calls items into it, however it simply doesn't sort the numbers/strings. The two methods are below, I don't know where the problem is. Numbers are randomly inputted.

CODE:

/**
     * Takes in entire vector, but will merge the following sections together:
     * Left sublist from a[first]..a[mid], right sublist from a[mid+1]..a[last].
     * Precondition: each sublist is already in ascending order
     *
     * @param a
     *            reference to an array of integers to be sorted
     * @param first
     *            starting index of range of values to be sorted
     * @param mid
     *            midpoint index of range of values to be sorted
     * @param last
     *            last index of range of values to be sorted
     */
    private void merge(ArrayList<Comparable> a, int first, int mid, int last) {
        int x;
        int i;
        ArrayList<Comparable> left = new ArrayList<Comparable>();
        ArrayList<Comparable> right = new ArrayList<Comparable>();
        mergeSort(a,first,mid);
        for(i = 0; i < a.size() - mid; i++){
            left.add(i,a.get(i));
            a.remove(i);
        }
        mergeSort(a,mid,last);
        for (x = mid; x < a.size(); x++) {
            right.add(x,a.get(x));
            a.remove(x);
        }
        if ((left.get(i).compareTo(right.get(x))) > 0) {
            i++;
            a.add(i);
        } else if (i < x) {
            x++;
            a.add(x);
        }


        System.out.println();
        System.out.println("Merge");
        System.out.println();

    }

    /**
     * Recursive mergesort of an array of integers
     *
     * @param a
     *            reference to an array of integers to be sorted
     * @param first
     *            starting index of range of values to be sorted
     * @param last
     *            ending index of range of values to be sorted
     */
    public void mergeSort(ArrayList<Comparable> a, int first, int last) {

        int mid = (first + last)/2;
        if(first == last){

        }else if(last - first == 1){
            merge(a,first, mid ,last);              
        }else{
            last = mid;
        }


                }

Answer 1

I have an outside class that calls items into it, however it simply doesn't sort the numbers/strings. The two methods are below, I don't know where the problem is.

The first problem is that if you call your mergeSort method with first = 0 and last = a.size() you won't sort anything as you only call merge if last-first == 1 :

public void mergeSort(ArrayList<Comparable> a, int first, int last) {
    int mid = (first + last)/2;
    if(first == last){
    }else if(last - first == 1){
        // you only merge if last - first == 1...
        merge(a,first, mid ,last);              
    }else{
        last = mid;
    }
}

Appart from this point, I don't get how you're trying to implement the Merge Sort algorithm. It's neither a top down, nor a bottom up implementation. You're splitting inside the merge method which is also really odd. It would have been easier to help you if you had provided your pseudo code + the way you call your public method. IMHO you have a real issue with your algorithm.

In fact the merge sort algorithm is really simple to implement. To illustrate this, I wrote this top down implementation of the merge sort algorithm using Deque instead of List objects:

import java.util.Deque;
import java.util.LinkedList;

public class Example {

    private LinkedList<Comparable> merge(final Deque<Comparable> left, final Deque<Comparable> right) {
        final LinkedList<Comparable> merged = new LinkedList<>();
        while (!left.isEmpty() && !right.isEmpty()) {
            if (left.peek().compareTo(right.peek()) <= 0) {
                merged.add(left.pop());
            } else {
                merged.add(right.pop());
            }
        }
        merged.addAll(left);
        merged.addAll(right);
        return merged;
    }

    public void mergeSort(final LinkedList<Comparable> input) {
        if (input.size() != 1) {
            final LinkedList<Comparable> left = new LinkedList<Comparable>();
            final LinkedList<Comparable> right = new LinkedList<Comparable>();
            // boolean used to decide if we put elements
            // in left or right LinkedList
            boolean logicalSwitch = true;
            while (!input.isEmpty()) {
                if (logicalSwitch) {
                    left.add(input.pop());
                } else {
                    right.add(input.pop());
                }
                logicalSwitch = !logicalSwitch;
            }
            mergeSort(left);
            mergeSort(right);
            input.addAll(merge(left, right));
        }
    }
}

I used Deque because peek()/ pop() is ways prettier IMHO than get(0) and remove(0) but it's up to you. If you absolutely want to use ArrayList here follows the corresponding implementation.

import java.util.ArrayList;
import java.util.List;

public class Example {

    private List<Comparable> merge(final List<Comparable> left, final List<Comparable> right) {
        final List<Comparable> merged = new ArrayList<>();
        while (!left.isEmpty() && !right.isEmpty()) {
            if (left.get(0).compareTo(right.get(0)) <= 0) {
                merged.add(left.remove(0));
            } else {
                merged.add(right.remove(0));
            }
        }
        merged.addAll(left);
        merged.addAll(right);
        return merged;
    }

    public void mergeSort(final List<Comparable> input) {
        if (input.size() != 1) {
            final List<Comparable> left = new ArrayList<Comparable>();
            final List<Comparable> right = new ArrayList<Comparable>();
            boolean logicalSwitch = true;
            while (!input.isEmpty()) {
                if (logicalSwitch) {
                    left.add(input.remove(0));
                } else {
                    right.add(input.remove(0));
                }
                logicalSwitch = !logicalSwitch;
            }
            mergeSort(left);
            mergeSort(right);
            input.addAll(merge(left, right));
        }
    }
}

Both implementation work with Integerand String or other Comparable.

Hope it helps.