2

Hey all I have been working on a merge sorter app with java in Eclipse. For some reason I can't figure it out and it is making me mad...

This is the main file I am working with

public class MergeSorter {

    public Integer[] sort(Integer[] array) {
        return mergesort(array);
    }


    private Integer[] mergesort(Integer[] array) {
      if(array.length > 1) {
          Integer[] left = new Integer[array.length /2];
          Integer[] right = new Integer[array.length - left.length];

          mergesort(left);
          mergesort(right);

          merge(left, right);
        }
        return array;
    }


    public Integer[] merge(Integer[] left, Integer[] right) {
        int leftI = 0;
        int rightI = 0;
        int newI = 0;
        Integer[] newArr = new Integer[left.length + right.length];

        while (leftI < left.length && rightI < right.length) {
            if (left[leftI] < right[rightI]) {
                newArr[newI] = left[leftI];
                leftI++;
            } else {
                newArr[newI] = right[rightI];
                rightI++;
            }
            newI++;
        }

        // Either the left or the right may still have elements
        for (int i = leftI; i < left.length; i++) { // left
            newArr[newI] = left[i];
            newI++;
        }
        for (int i = rightI; i < right.length; i++) { // right
            newArr[newI] = right[i];
            newI++;
        }
        return newArr;
    }


    public void printCollection(Integer[] items) {
        for (int i = 0; i < items.length; i++) {
            System.out.print(items[i] + " ");
        }
        System.out.println();
    }
}

There is also a driver file that has everything done it it to run this code. I was told I need to genercize the merger sorter class to using arrayLists rather than arrays, but I am lost on how to do so. I am thinking this is where i need to use the List ? but not sure on it, any help is appreciated

Smittey
  • 2,475
  • 10
  • 28
  • 35
c0212
  • 45
  • 6
  • Is there a reason you are using the boxed `Integer` type instead of the primitive `int`? – Frithjof Nov 08 '15 at 22:42
  • 1
    What error does it give you? – valepu Nov 08 '15 at 22:44
  • 1
    Identify the problem (E.g.: you can test that `private` method before integration - beause there is nothing in `left` and `right` when you call `mergesort`) and provide an input, your actual output and the expected one. – ROMANIA_engineer Nov 08 '15 at 22:45
  • Try using `valueOf()` in comparisons, that is try to change `left[leftI] < right[rightI]` into `left[leftI].valueOf() < right[rightI].valueOf()` – valepu Nov 08 '15 at 22:51
  • @valepu: That shouldn't matter. Boxing/unboxing is automatic for < and >. – President James K. Polk Nov 08 '15 at 22:52
  • 1
    Time to learn how to debug. – m0skit0 Nov 08 '15 at 22:53
  • All of your current code problems are in the `mergesort(...)` method. In particular, you are ignoring the values returned by recursive calls to `mergesort` and also calls to `merge`. You need to correctly utilise the values returned here. Secondly, `mergesort` creates two new arrays, but does not fill them with values. You need to add some code to copy the relevant pieces of `array` into `left` and `right`. – clstrfsck Nov 08 '15 at 23:01
  • I am getting errors on the merge sort method, telling me that merge sorter class is not generic. – c0212 Nov 08 '15 at 23:35
  • This is my first time attempting to work with merge sort from my class I am in and just trying to learn it on my own, so I know what to do on our projects, and completely lost inside the merge sort method. Not sure what I need to do to make it generic and what needs returned and whatnot inside the merge sort method – c0212 Nov 08 '15 at 23:36

1 Answers1

2

There two major wholes in your logic. First, your mergesort returns it's input no matter what. Second, your merged array is never stored.

private Integer[] mergesort(Integer[] array) {
  if(array.length > 1) {
      Integer[] left = new Integer[array.length /2];
      Integer[] right = new Integer[array.length - left.length];

      mergesort(left); //problem 1.5: more on this later...
      mergesort(right);

      merge(left, right); //problem 2: stored array is returned but not stored
    }
    return array; //problem 1: returns array unchanged
}

Solving problem 2 is simple. We just need to assign the output to a field like so:

Integer[] result = merge(left,right);

or

array = merge(left,right);

Using either or these, result or array will not retain the work done in merge(). The strategy you choose will depend whether or not you want to modify your input.

The relationship may actually be counter intuitive, but if you want your input to remain unchanged you can use the second line and now also problem 1 is solved because we've changed array. (The input is unchanged because Java is pass-by-value meaning array is a new field with the same array as your input, not the same field containing that array, thus assignment makes puts a separate array in the field array without touching the original). Unfortunately this solution leaves us with problem 1.5, because we're not changing left and right by calling mergesort(), in which case we need to store as we did the results of merge().

If you want to mergesort() to change the input array (which is not an unreasonable idea) then we need to create the separate Integer[] results given above and copy it's values to the array held int array. This also adverts problem 1.5 but may change the intent of you method.

EDIT: There is a third problem I neglected to mention. In mergesort() you've created left and right but they are never filled. You should copy the values from array into your left or right halves. Something like:

for(int i=0; i<left.length; i++) left[i] = array[i];
for(int i=0; i<right.length; i++) right[i] = array[left.length+i];


Now to answer you're second question, it should not be hard to do every thing you've done with an ArrayList<Integer>. The documentation list all the methods and you can treat an ArrayList as array often (execpt instead of array[i] or array[i]=value you have to use methods array.get(i) and array.set(i,value)). However you can simplify things by using methods like array.add(value) which puts a value at the end of the list so that you don't have to keep up with where the end actually is.

Hopefully this helps you understand exactly where your problems lie and resolve them so your brain can itch over all the other wonderful details programming provides

Linus
  • 894
  • 7
  • 13
  • Thank you so much for the help! Helped out a ton for me. My last thing I am stuck on is just the making it generic by the ArrayLists. Will I have to change the whole method name to each one from Integer[] merge sort(.....); to something completely different? – c0212 Nov 09 '15 at 00:45
  • Yes. Instead of `Integer[]` you will need to use `ArrayList`. They implement similar behavior but are not interchangeable. – Linus Nov 09 '15 at 01:01
  • Alright, I am working on that now. But when I go ahead and change it, I than get errors with anything that is array.length for the left and right and whatnot in my merge sort method. This is where I am getting completely lost.. – c0212 Nov 09 '15 at 01:05
  • `ArrayList` needs to use it's own methods. Read the documentation to know what each does and figure out which you need. IF you can't make something out of it you can try searching for third party "ArrayList tutorials". Java's own [tutorials](http://docs.oracle.com/javase/tutorial/collections/interfaces/collection.html) cover it in abstractions like `collection` and `list` but may be worth a look too. Good luck. – Linus Nov 09 '15 at 01:21
  • @np0212, one more thing I missed has been added. My apologizes if it left you with any problems. – Linus Nov 09 '15 at 02:15
  • I appreciate the help! I got it most of the way done, just not running exactly how it is supposed to, I appreciate it! – c0212 Nov 09 '15 at 02:48