Sorting an integer array in java, using selection sort. Cannot find my error

Question

I'm trying to learn code by making 'easy' exercises. I am trying to make a search algorithm using selection sort. When I follow the code inside my head it makes perfect sense, but when I run it, it does not order anything. For the array I am using an integer only array, it consist of random numbers and is of random length.

int currentMin;
    int currentMinIndex = 0;
    int temp;
    for(int i=0;i<array.length-1;i++){
        currentMin = array[i]; 
        for(int j=i+1;j<array.length-1;j++){
            if(array[j]<currentMin){
                currentMinIndex = j; 
                currentMin = array[j];
            }
        }
        temp = array[currentMinIndex]; //I am aware this could be currentMin 
        array[currentMinIndex] = array[i];
        array[i] = temp;
    }

I hope someone will spot my mistake and tell me. (If you have other 'easy' exercises I could do, creativity is appreciated, however this post must stay on topic)

Edit: I just noticed that in some weird way when the array is of large length it sorts but the last one. (array length vary because they are random)

Have I got news for you! You don't have to follow code in your head anymore. — shmosel
– shmosel, Commented Dec 15, 2016 at 23:08
The inner loop needs to go to array.length (remove the -1) — castletheperson
– castletheperson, Commented Dec 15, 2016 at 23:11
I have already wrote it down, followed it writing every variable down. All warnings are on. I think I already broke down the code, idk how to further. I've done the testcasing part. I'm sorry for just being silly. — Marekkk
– Marekkk, Commented Dec 15, 2016 at 23:13
Also, you have two variables currentMin and currentMinIndex, where currentMinIndex would be sufficient. And if you set currentMin to array[i], then you must also set currentMinIndex to i. All this would be very apperent if you stepped through your code using a debugger. — JB Nizet
– JB Nizet, Commented Dec 15, 2016 at 23:15
@4castle Okay that worked out a bit, but I am still getting unsorted arrays. I have just changed the length to 5 so could see it better. but sometimes it is sorting and other times its not. And I'm using totally random numbers... — Marekkk
– Marekkk, Commented Dec 15, 2016 at 23:15

Elliott Frisch · Accepted Answer · 2016-12-15 23:16:25Z

2

You need to update currentMinIndex to i when you set currentMin, and your inner loop should be to array.length.

currentMin = array[i]; 
currentMinIndex = i;
for(int j=i+1;j<array.length;j++){

You could further simplify this by moving your declarations into the loop and removing temp (as you have currentMin) like

for (int i = 0; i < array.length - 1; i++) {
    int currentMin = array[i];
    int currentMinIndex = i;
    for (int j = i + 1; j < array.length; j++) {
        if (array[j] < currentMin) {
            currentMinIndex = j;
            currentMin = array[j];
        }
    }
    array[currentMinIndex] = array[i];
    array[i] = currentMin;
}

answered Dec 15, 2016 at 23:16

Elliott Frisch

202k20 gold badges166 silver badges265 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

leeyuiwah · Accepted Answer · 2016-12-15 23:24:16Z

There were two problems

You forgot to reset currentMinIndex
The boundary condition in the inner for loop was not right

Here is a modified version of your code:

public class SelectionSort {

    public static void main(String[] args) {
        int currentMin;
        int currentMinIndex = 0;
        int temp;
        int[] array = {9, 1, 3, 2, 4, 7};
        for(int i=0;i<array.length-1;i++){  
            currentMin = array[i]; 
            currentMinIndex = i;                    // Problem #1
            for(int j=i+1;j<=array.length-1;j++){   // Problem #2
                if(array[j]<currentMin){
                    currentMinIndex = j; 
                    currentMin = array[j];
                }
            }
            temp = array[currentMinIndex]; //I am aware this could be currentMin 
            array[currentMinIndex] = array[i];
            array[i] = temp;
        }

        StringBuilder sb = new StringBuilder();
        sb.append("[");
        String comma = "";      // first time special
        for (int i=0; i<array.length; i++) {
            sb.append(comma + array[i]);
            comma = ", ";       // second time and onwards
        }
        sb.append("]");
        System.out.println(sb.toString());
    }
}

The output of this program is

[1, 2, 3, 4, 7, 9]

Abhishek Patel · Accepted Answer · 2016-12-15 23:12:55Z

0

Both your loops are skipping the last element. Consider using <= instead of < in your loops conditions to consider the last element as well.

answered Dec 15, 2016 at 23:12

Abhishek Patel

8147 silver badges21 bronze badges

Collectives™ on Stack Overflow

Sorting an integer array in java, using selection sort. Cannot find my error

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related