apebeast
apebeast

Reputation: 11

Strange selection sort behavior

I have tried to attempt implementing my own Selection Sort code. So far, the code sorta works... It just behaves strangely.

class HelloWorld {
 public static void main(String[] args) {
    long startTime = System.currentTimeMillis();

    int a[] = {
        3,7,8,4,22,13,9,5,10
    };

    int max_pos = a.length-1;
    int counter = 0;
    int min_sort = 0;
    int j = -1;
    int min_pos = 0;
    int i=0;

    while (j < max_pos) {
        i=j+1;
        min_sort = a[i];
        // traverse trough unsorted portion
        while (i < max_pos) {
            i++;
            if (min_sort > a[i]) {
                min_sort  = a[i]; // minimum unsorted
                min_pos = i;
            }
        }

        // sorted portion
        j++;
        a[min_pos] = a[j];
        a[j] = min_sort ;
        for(int x=0; x < a.length; x++) {
            System.out.print(a[x] + ", ");
        }
        System.out.print("\t\t");
        System.out.println("i: " + i + " j: " + j + " min pos: " + min_pos + " min val: "+ min_sort);
    }
  }
 }

I have tried tracing it, and the output goes like this:

3, 7, 8, 4, 22, 13, 9, 5, 10,           i: 8 j: 0 min pos: 0 min val: 3          
3, 4, 8, 7, 22, 13, 9, 5, 10,           i: 8 j: 1 min pos: 3 min val: 4          
3, 4, 5, 7, 22, 13, 9, 8, 10,           i: 8 j: 2 min pos: 7 min val: 5          
3, 4, 5, 7, 22, 13, 9, 7, 10,           i: 8 j: 3 min pos: 7 min val: 7          
3, 4, 5, 7, 7, 13, 9, 22, 10,           i: 8 j: 4 min pos: 7 min val: 7          
3, 4, 5, 7, 7, 9, 13, 22, 10,           i: 8 j: 5 min pos: 6 min val: 9          
3, 4, 5, 7, 7, 9, 10, 22, 13,           i: 8 j: 6 min pos: 8 min val: 10         
3, 4, 5, 7, 7, 9, 10, 13, 22,           i: 8 j: 7 min pos: 8 min val: 13         
3, 4, 5, 7, 7, 9, 10, 13, 22,           i: 8 j: 8 min pos: 8 min val: 22

Upvotes: 0

Views: 38

Answers (1)

paxdiablo
paxdiablo

Reputation: 881463

Since this is almost certainly an educational process on your part (real code would just call Arrays.sort()), I won't give a direct answer initially.

Instead, you should notice that it's the line where j == 3 which is when the 8 appears to magically transmogrify into a 7.

So you should start to hone your debugging skills. Run the code through a debugger up to the end of the output of the previous line, then single step every single instruction. At some point, you'll see the 7 become an 8 and that's the code you need to focus on.

Once you've done that, you'll hopefully be able to tell what the problem is. However, if you're still stuck (make sure you try first, or you'll never improve), see below. t---

This is a detailed analysis of your problem. You are storing information about the minimum (both the value min_sort and its position min_pos) each time you find an element that's smaller than the first one you set up in each iteration of the outer loop:

if (min_sort > a[i]) {
    min_sort  = a[i]; // minimum unsorted
    min_pos = i;
}

Now that's a good way to do it, and these two pieces of information are later used in the swapping process(1):

j++;
a[min_pos] = a[j];
a[j] = min_sort;

However, what of the case where you don't find an element smaller than the first one set up, such as the following where when position 3 holds the value 7 and all values beyond that are greater than 7:

3, 4, 5, 7, 22, 13, 9, 8, 10
         ^

In that case, the body of the if statement will never be run and the initial conditions from the start of the iteration will still apply:

i=j+1;
min_sort = a[i];

Notice anything missing there? Such as, let's see, the position being set to something?

Bingo! If the first element being checked in an iteration is already in its correct position, min_pos will remain set to whatever it was previously and, when you swap, that's not going to be pretty. You can fix that by changing the code immediately inside the outer while loop to be:

i = j + 1;
min_pos = i;          // add this line
min_sort = a[i];

and you'll see the greatly improved (i.e., correct) output of:

3, 7, 8, 4, 22, 13, 9, 5, 10, i: 8 j: 0 min pos: 0 min val: 3
3, 4, 8, 7, 22, 13, 9, 5, 10, i: 8 j: 1 min pos: 3 min val: 4
3, 4, 5, 7, 22, 13, 9, 8, 10, i: 8 j: 2 min pos: 7 min val: 5
3, 4, 5, 7, 22, 13, 9, 8, 10, i: 8 j: 3 min pos: 3 min val: 7
3, 4, 5, 7, 8, 13, 9, 22, 10, i: 8 j: 4 min pos: 7 min val: 8
3, 4, 5, 7, 8, 9, 13, 22, 10, i: 8 j: 5 min pos: 6 min val: 9
3, 4, 5, 7, 8, 9, 10, 22, 13, i: 8 j: 6 min pos: 8 min val: 10
3, 4, 5, 7, 8, 9, 10, 13, 22, i: 8 j: 7 min pos: 8 min val: 13
3, 4, 5, 7, 8, 9, 10, 13, 22, i: 8 j: 8 min pos: 8 min val: 22

(1) The swap, by the way, is not necessary when the smallest remaining value is already in the correct position, though it does no harm. If you wanted to avoid the unnecessary swap, you could use:

j++;
if (min_pos != j) {
    a[min_pos] = a[j];
    a[j] = min_sort ;
}

However, as stated, it's not a problem to do the swap so it's really up to you.

Upvotes: 1

Related Questions