Why is my swap method not working?

Question

I have successfully written a Bubblesort class. Now I am trying to break the functions apart to make the code cleaner. Here is the working code:

public class BubbleSortTest {

    public static void main(String[] args) {
        int[] array = {93, 60, 65, 45, 50};
        bubbleSort(array);
        printArray(array);
    }

    public static int[] bubbleSort(int[] array) {
        for(int i = 0; i < array.length - 1; i++) {
            for(int j = 0; j < array.length - 1 - i; j++) {
                if(array[j] < array[j + 1]) {
                    int temp = array[j];
                    array[j] = array[j + 1];
                    array[j + 1] = temp;
                 }
             }
         }
         return array;
    }

    public static void printArray(int[] array) {
        for(int i = 0; i < array.length; i++) {
            System.out.print(array[i] + " ");
        }
    }
}

For this example, I want to take the functionality of the if statement in the bubbleSort function and make it into its own swap function. So the code I want would look like:

public class BubbleSortTest {

    public static void main(String[] args) {
        int[] array = {93, 60, 65, 45, 50};
        bubbleSort(array);
        printArray(array);
    }

    public static int[] bubbleSort(int[] array) {
        for(int i = 0; i < array.length - 1; i++) {
            for(int j = 0; j < array.length - 1 - i; j++) {
                if(array[j + 1] > array[j]) {
                    swap(array[j + 1], array[j]);
                }
             }
         }
         return array;
    }

    public static void swap(int largerElement, int smallerElement) {
        int temp = largerElement;
        largerElement = smallerElement;
        smallerElement = temp;
    }

    public static void printArray(int[] array) {
        for(int i = 0; i < array.length; i++) {
            System.out.print(array[i] + " ");
        }
    }
}

When I run the latter code, swap is essentially doing nothing. I thought a fix to this was to create a non-void function to return the values, but I can't return both values without encapsulating them in an object (correct me if I'm wrong). There must be something wrong with the swap function. I'm probably missing something fundamental. What's wrong with the latter code?

http://stackoverflow.com/questions/2393906/how-do-i-make-my-swap-function-in-java — assylias, Feb 27 '15 at 15:09

score 3 · Accepted Answer · edited May 23 '17 at 12:02

3

Java is Pass By Value. Your swap method must receive the array and the indexes to swap rather than the int variables:

public static void swap(int[] array, int index1, int index2) {
    int temp = array[index1];
    array[index1] = array[index2];
    array[index2] = temp;
}

edited May 23 '17 at 12:02

Community

1
1

answered Feb 27 '15 at 15:09

Luiggi Mendoza

85,076
16
154
332

Hmm that's interesting, but makes sense. The function call would look like swap(array, j, j + 1);. – BrianRT Feb 27 '15 at 15:16
@daChi in this case, yes. – Luiggi Mendoza Feb 27 '15 at 15:18
I also find it hard to find existing answers to questions (like mine) without knowing exactly what to ask for, or exactly where the problem lies, or exactly what fundamental is being misunderstood. Hence, it's difficult to not ask duplicate questions. – BrianRT Feb 27 '15 at 15:21
+1 for Pass-By-Value. But I don't think this `swap` makes the code any cleaner (I know a classic `swap()` isn't even possible in Java)... – Axel Feb 27 '15 at 15:26
@Axel the fact it makes or not the code more readable falls into personal perspective. For me, it does because the code can be read similarly to the algorithm. – Luiggi Mendoza Feb 27 '15 at 15:27
I am reading "Clean Code" by Robert C. Martin, and it said that functions should do ONE thing really well. So, usually shorter is better. That's the reason why I wanted to add the swap function--because bubbleSort should just sort, and swap should just swap. Plus, it makes the code more reusable. – BrianRT Feb 27 '15 at 15:54
Yes, that's alright, and I would use a `swap()` method in C++ where it can be implemented easily (and IIRC it already is in the library). But in this case (Java), the benefit is really small, and if you change your bubblesort to work on other data (like `ArrayList`), you have to change `swap()` as well. – Axel Feb 27 '15 at 16:03
@Axel you can have a swap method for Java by using a small *hack* as shown here: http://stackoverflow.com/a/20600020/1065197 – Luiggi Mendoza Feb 27 '15 at 16:04
@LuiggiMendoza now that gives me the creeps. Only use case I can see is to confuse the person that will have to maintain your code later... ;-) – Axel Feb 27 '15 at 17:29
1

@Axel but your requirement is fulfilled ;) – Luiggi Mendoza Feb 27 '15 at 17:30

Why is my swap method not working?

1 Answers1