I am trying to write a selection sort, but I am getting a outofbounds error. How do I fix this?

public class SelectionSort{

    public static void selectSort(int[] arr){
        int least = 0;
        for(int i = 1; i < arr.length - 1; i++){
            for(int j = i + 1 ; i  < arr.length-1; j++){
                if(arr[j] < arr[least]){
                    int temp = arr[j];
                    arr[least] = arr[j];
                    arr[j] = temp;
                    least = j;
                }
            }
        }
    }

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

    public static void main(String[] args){
        int[] array = {2,59,1,3,8,7,4};
        selectSort(array);
        System.out.println("\n\n\n\n =========================\n");
        printArray(array);
    }

}
for(int i = 1; i < arr.length - 1; i++){
    for(int j = i + 1 ; i < arr.length-1; j++){

You are testing i < arr.length - 1 in the inner loop when you probably want to be testing j < arr.length - 1. Except that you probably want to include the last element of the array in your loops, so that would really be i < arr.length and j < arr.length.

Also, that's not how you swap the values of arr[j] and arr[least]; you are wiping out the current value of arr[least] and making arr[j] and arr[least] equal. Notice that temp = arr[j] so there's really no point in going arr[j] = temp when they are already equal.

it seems to be mistake in your inner loop as well as in sorting logic.
Condition for inner loop should check with j instead of i. like j<arr.length

for(int i = 1; i < arr.length - 1; i++){
            for(int j = i + 1 ; i  < arr.length-1; j++){

even i don't understand why you have started loop with i=1 and upto arr.length-1.
can't you start loop through i=0?
because when you start it from i=1,you are skipping 1st element of the array.

Actually there is also no need of least variable...

and you are doing temp = arr[least] and arr[least] = temp.
Here swapping is not performed at all.also you ahve written least = j;

you have to start your outer loop with i=0 and upto i<arr.length and same with inner loop.
not lenghth-1 because it skips last element in the array.

instead of using least variable use i and j,for swaping,and last write j=i;

Thanks guys for the help, and here is the full code now working:

public class SelectionSort{

    public static void selectSort(int[] arr){
        for(int i = 0; i < arr.length; i++){
            for(int j = 0 ; j  < arr.length; j++){
                if(arr[j] > arr[i]){
                    int temp = arr[i];
               arr[i] = arr[j];
               arr[j] = temp;
                }
            }
        }
    }

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

    public static void main(String[] args){
        int[] array = {2,59,1,3,8,7,4};
        printArray(array);
        selectSort(array);
        System.out.println("\n\n\n\n =========================\n");
        printArray(array);
    }

}

Edited 3 Years Ago by godzab

This article has been dead for over six months. Start a new discussion instead.