what is the error in this logic of selection sort

Question

the first for loop will start at the end and swap its value to the max value in the array a[0-->its own index]

int selection_sort(int *a, int size) {
    int i, j, swap;

  
   
    for (i = size - 1; i <= 0; i--) {
        /*this will find the max between o index to i th index*/
        for (j = 0; j <= i; j++) {
            if (a[i] < a[j]) {
                swap = a[i];
                a[i] = a[j];
                a[j] = a[i];
            }
        }
    }
}

output is the same array with no change in order

Please take the [tour] and read [ask]. You haven't included a question in your question, and it's not clear what your problem is. I will note that you don't use the value put into `swap`. — Thomas Jager, Jun 30 '22 at 15:20
@Shubham Gaikwad This for loop for (i = size - 1; i <= 0; i--) { never iterates provided that size is not less than 2. — Vlad from Moscow, Jun 30 '22 at 15:22
What research have you done? Have you compared your implementation against the example(s) found on [Selection sort - Wikipedia](https://en.wikipedia.org/wiki/Selection_sort)? — Oka, Jun 30 '22 at 15:23
This seems like a good time to learn how to [*debug*](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) your programs. More specifically how to use a [*debugger*](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) to step through the code statement by statement while monitoring your variables and their values. — Some programmer dude, Jun 30 '22 at 15:23
Though in this case perhaps using a debugger isn't needed, if you successfully manage to explain your code to your [rubber duck](https://en.wikipedia.org/wiki/Rubber_duck_debugging). — Some programmer dude, Jun 30 '22 at 15:24
A conventional selection sort would do only one swap (at most) per pass through the unsorted part of the array. Not that that's related to why your function seems not to modify the order of the array elements. — John Bollinger, Jun 30 '22 at 15:37

Vlad from Moscow · Answer 1 · 2022-06-30T15:48:42.513

For starters the function has the return type int but returns nothing. Also the second parameter should have the unsigned integer type size_t instead of the signed integer type int.

Also you should declare variables in minimum scopes where they are used.

The main problem with your code is that the body of this for loop

for (i = size - 1; i <= 0; i--) {

will never get the control provided that size is not less than 2.

Also there is a typo

a[j] = a[i];

You have to write

a[j] = swap;

Apart from this there are redundant swaps.

The function can look the following way

void selection_sort( int *a, size_t n ) 
{
    for ( size_t i = n; i-- != 0; ) 
    {
        size_t max = i;

        for ( size_t j = 0; j < i; j++ ) 
        {
            if ( a[max] < a[j] ) max = j;
        }

        if ( max != i )
        {
            int swap = a[max];
            a[max] = a[i];
            a[i] = swap;
        } 
    }
}

Here is a demonstration program.

#include <stdio.h>

void selection_sort( int *a, size_t n ) 
{
    for ( size_t i = n; i-- != 0; ) 
    {
        size_t max = i;

        for ( size_t j = 0; j < i; j++ ) 
        {
            if ( a[max] < a[j] ) max = j;
        }

        if ( max != i )
        {
            int swap = a[max];
            a[max] = a[i];
            a[i] = swap;
        } 
    }
}

int main( void )
{
    int a[] = { 9, 8, 7, 6, 5, 4, 3, 2, 1, 0 };
    const size_t N = sizeof( a ) /sizeof( *a );

    for ( size_t i = 0; i < N; i++ )
    {
        printf( "%d ", a[i] );
    }
    putchar( '\n' );

    selection_sort( a, N );

    for ( size_t i = 0; i < N; i++ )
    {
        printf( "%d ", a[i] );
    }
    putchar( '\n' );
}

Its output is

9 8 7 6 5 4 3 2 1 0 
0 1 2 3 4 5 6 7 8 9

You modify the algorithm quite considerably. Besides the problems you mentioned, what was wrong with the OP's algorithm? — Some programmer dude, Jun 30 '22 at 15:37
@Someprogrammerdude The main problem is that its function sorts nothing.:) — Vlad from Moscow, Jun 30 '22 at 15:39
Well yes of course... :) But shouldn't fixing the loop and the swap solve that? Why the need to rewrite the whole function to be very different from what the OP have? Why is it better? :) — Some programmer dude, Jun 30 '22 at 15:42
@Someprogrammerdude I have removed redundant swaps and have made less typos.:) — Vlad from Moscow, Jun 30 '22 at 15:50

score 2 · Answer 2 · answered Jun 30 '22 at 15:30

Your for loop is wrong for (i = size - 1; i <= 0; i--). The comparison should be >=0.

Also, the value swapping is incorrect

            if (a[i] < a[j]) {
                swap = a[i];
                a[i] = a[j];
                a[j] = a[i];
            }

should be

            if (a[i] < a[j]) {
                swap = a[i];
                a[i] = a[j];
                a[j] = swap;
            }

what is the error in this logic of selection sort

2 Answers2