Selection sort growing ordered ranges from both ends

Question

I have written a modified version of selection sort where I consider both a minimum and maximum of an array and place them at the two ends

The algorithm works like this

1. Find the minimum and the maximum value in the list.
2. Swap the minimum value with the value in the first position.
3. Swap the maximum value with the value in the last position.
4. Repeat the steps above for the remainder of the list 
(starting at the second position and ending at the second to 
 last position and narrowing the range of positions examined 
 from both ends of the array each time).

Unfortunately the above is showing unexpected results for arrays having duplicates values.

For example,

[9, 37, 12, 1, 13, 31, 5, 37, 36, 29, 19, 22, 20, 15, -1, 23]

was sorted to

[-1, 1, 5, 9, 12, 13, 15, 19, 20, 22, 23, 29, 31, 37, 36, 37]

In fact, the main issue here is that the algorithm in general is not doing proper sorting for the elements in the latter part of the array, besides simply with respect to duplicates.

Here is my pseudocode

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

       if (arr[j] <=arr[minIndex]) {
         minIndex = j;      
         } 
       if(arr[j]>=arr[maxIndex]){
          maxIndex = j; 
         }
      }
      swap(arr, i, minIndex);
      swap(arr, (arr.length-i-1), maxIndex); 
    i++;
    }

EDIT This is the swap part of my code which is the only thing that interacts with the algorithm. I don't think it will make any difference but I'll include it anyway

 private static void swap(int[] arr, int oldIndex, int newIndex){

    int temp=arr[oldIndex];
    arr[oldIndex]=arr[newIndex];
    arr[newIndex]=temp;
 }

Answer 1

The problem happens when i happens to be the maxIndex. To fix that you need to add:

swap(arr, i, minIndex);
if(i == maxIndex) {
     maxIndex = minIndex;
}
swap(arr, (arr.length-i-1), maxIndex);

See it @work

Answer 2

OK, the problem is the case where the maximum value starts in the minimum position of the iteration. Consider the second time through the loop on your problem array:

-1,37,12,1,13,31,5,23,36,29,19,22,20,15,9,37

i is 1, len-i-1 is 14. After the loop, maxindex is 1, and minIndex is 3.

So you swap 1 (i) and 3 (minIndex):

-1,1,12,37,13,31,5,23,36,29,19,22,20,15,9,37

And then 14 (len-i-1) and 1 (maxIndex):

-1,9,12,37,13,31,5,23,36,29,19,22,20,15,1,37

Oops. Basically, you need to do both swaps in parallel.

EDIT Parallelism won't really help in the case of two overlapping swaps, since each of the swaps wants to put a different value in one of the array slots; you have to resolve the conflict. I think @codaddict's solution works well.

Answer 3

You have an off-by-one error in the termination condition of the inner for loop..

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

that should be a <, not a <=, for the same reason you start with i+1 instead of i.

As a stylistic recommendation, I would also store the value arr.length - i - 1 in a variable, since it shows up four times in the code.

EDIT: verified this error is not the source of the problem.

Answer 4

I think you're doing too many swaps - i.e. you're doing the swaps regardless of whether you've actually found a new min or max value. Also, you don't need to do both tests (of min and max, as you cannot be both, except in the trivial case of equality, in which case it doesn't matter which order they're in...).

I submit that the following is a better representation of your algorithm:

int arrayLength = arr.length;
for (int i=0; i<arrayLength; i++){
  for (int j=i+1; j<arrayLength; j++){
    int v = arr[j];
    if (v < arr[i]){
      swap(arr, i, j);
    } else if (v > arr[(arrayLength -1)]){
      swap(arr, (arrayLength -i-1), j);
    }
  }
}

However, you don't actually need to do the tests on the max values to swap them to the end, as these will all be caught by the search for min values as you do you top-level iteration through your array - i.e.

int arrayLength = arr.length;
for (int i=0; i<arrayLength; i++){
  for (int j=i+1; j<arrayLength; j++){
    int v = arr[j];
    if (v < arr[i]){
      swap(arr, i, j);
    } 
  }
}

would be more efficient.

EDIT: Having reviewed the other answers, this sort of picks up on Mark Reed's answer about doing the swaps in parallel - except that, I'd suggest you do the swaps as you've found the need to.

Answer 5

Well as in the comments, your first swap will do well, but your second swap makes an issue.

In the case when your maxindex variable was located at i, you placed your minimum term correctly, but lost your max term. This can be tackled by adding an extra provision that checks if(i==maxterm), and handle this special case by making first swap normally and second swap of maxterm and minterm locations secondly. It is useless for you to traverse through your array from 0 to array.length-1

I recommend, traverse from 0 to (array.length-1)/2, as you are sorting on both side. The end part is already sorted as is the starting part, why make extra iterations ? I have written an exactly same program with corrections I suggested which also checks if the numbers are already sorted and terminates loop immediately.