HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Bubble sorting an int array

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
arraysortingbubbleint

Problem

I'd like to improve this bubble sort code

package com.arun.sort;

 import java.util.Arrays;

 public class BubbleSort {

public static void main(String[] args) {

    int[] arr={2,5,1,8,12,3,7};
    int n=arr.length;

    for(int k=0;karr[i+1]){
                int temp=arr[i];
                arr[i]=arr[i+1];
                arr[i+1]=temp;
            }
        }
    }

    System.out.println("Sorted array" +Arrays.toString(arr));

  }

   }

Solution

Step 1: format it nicely

An IDE can automatically reformat code nicely to follow the standard:

int n = arr.length;

for (int k = 0; k  arr[i + 1]) {
            int temp = arr[i];
            arr[i] = arr[i + 1];
            arr[i + 1] = temp;
        }
    }
}


In Eclipse it's Control-Shift-f. It adds spaces around operators. Now the code looks like the standard, and it's easier to read and code review for everyone.

Step 2: extract to a method

In the current code you have a hardcoded array, and the main logic follows right after. It's hard to test this way. What if you want to see if the implementation works with a different set of numbers? You have to rewrite the array. Better to extract the main logic into its own, independent method:

void sort(int[] arr) {
    // ...
}


Now you can test with multiple different inputs easier:

arr = new int[]{2, 5, 1, 8, 12, 3, 7};
BubbleSort.sort(arr);
System.out.println(Arrays.toString(arr));

arr = new int[]{4, 3, 2, 1, 2};
BubbleSort.sort(arr);
System.out.println(Arrays.toString(arr));


Step 3: convert print statements to proper unit tests

The problem with print statements is that every time you change something and rerun, you have to re-verify the output of each statement. Unit tests can automate the verification step, and converting is easy enough to do:

@Test
public void testMixedValues() {
    int[] arr = {2, 5, 1, 8, 12, 3, 7};
    BubbleSort.sort(arr);
    assertEquals("[1, 2, 3, 5, 7, 8, 12]", Arrays.toString(arr));
}

@Test
public void testDecreasingValues() {
    int[] arr = {4, 3, 2, 1};
    BubbleSort.sort(arr);
    assertEquals("[1, 2, 3, 4]", Arrays.toString(arr));
}

@Test
public void testDecreasingWithDups() {
    int[] arr = {4, 3, 2, 1, 2, 3, 2};
    BubbleSort.sort(arr);
    assertEquals("[1, 2, 2, 2, 3, 3, 4]", Arrays.toString(arr));
}


Now you can make changes and the test cases will flag an error if something breaks. Unless you do something really horrible, typically only a few of the test cases will break, and you don't need to reverify the others that are still working, which makes debugging a lot easier.

Minor things

Instead of n, a better name would be length to cache the length of the array.

As for the loop variables, it's more traditional to name nested counter variables as i, j, k, in this order of nesting level, instead of k, i as you did. In any case, this is really not a big deal.

int length = arr.length;

for (int i = 0; i  arr[j + 1]) {
            int temp = arr[j];
            arr[j] = arr[j + 1];
            arr[j + 1] = temp;
        }
    }
}

Code Snippets

int n = arr.length;

for (int k = 0; k < n - 1; k++) {
    for (int i = 0; i < n - k - 1; i++) {
        if (arr[i] > arr[i + 1]) {
            int temp = arr[i];
            arr[i] = arr[i + 1];
            arr[i + 1] = temp;
        }
    }
}
void sort(int[] arr) {
    // ...
}
arr = new int[]{2, 5, 1, 8, 12, 3, 7};
BubbleSort.sort(arr);
System.out.println(Arrays.toString(arr));

arr = new int[]{4, 3, 2, 1, 2};
BubbleSort.sort(arr);
System.out.println(Arrays.toString(arr));
@Test
public void testMixedValues() {
    int[] arr = {2, 5, 1, 8, 12, 3, 7};
    BubbleSort.sort(arr);
    assertEquals("[1, 2, 3, 5, 7, 8, 12]", Arrays.toString(arr));
}

@Test
public void testDecreasingValues() {
    int[] arr = {4, 3, 2, 1};
    BubbleSort.sort(arr);
    assertEquals("[1, 2, 3, 4]", Arrays.toString(arr));
}

@Test
public void testDecreasingWithDups() {
    int[] arr = {4, 3, 2, 1, 2, 3, 2};
    BubbleSort.sort(arr);
    assertEquals("[1, 2, 2, 2, 3, 3, 4]", Arrays.toString(arr));
}
int length = arr.length;

for (int i = 0; i < length - 1; i++) {
    for (int j = 0; j < length - i - 1; j++) {
        if (arr[j] > arr[j + 1]) {
            int temp = arr[j];
            arr[j] = arr[j + 1];
            arr[j + 1] = temp;
        }
    }
}

Context

StackExchange Code Review Q#58178, answer score: 14

Revisions (0)

No revisions yet.