Why aren't my for loops working in this bubble sort function in C

82 Views Asked by At

I was having this problem to which I have come to the solution through the trial and error process but I have no idea why my bubble sort function wasn't working in the first place.

The problem had to do with the for-loops inside my function. Specifically when declaring and defining my i and j variables.

In my version of C I can define variables inside my for-loop parameter, but I can't declare them, so I do both the declaration and definition outside.

Doing so though made my function not work as intended as it didn't sort my array at all.

Though after declaring the variables outside but defining them inside the for-loop parameter to my surprise the function worked properly. My problem is I have no idea why.

Here I am providing both the working version and the non-working version:

Non-Working Version:

void bubbleDesc (int n, int array[])
{
  
  int i = 0, j = 0, temp;
  
  for (i; i < n - 1; i++)
  {
    for (j; j < n - 1; j++)
    {
      if (array[j] < array[j + 1])
      {
        temp = array[j + 1];
        array[j + 1] = array[j];
        array[j] = temp;
      }
    }
  }
  
}

Working Version:

void bubbleDesc (int n, int array[])
{
  
  int i, j, temp;
  
  for (i = 0; i < n - 1; i++)
  {
    for (j = 0; j < n - 1; j++)
    {
      if (array[j] < array[j + 1])
      {
        temp = array[j + 1];
        array[j + 1] = array[j];
        array[j] = temp;
      }
    }
  }
  
}
2

There are 2 best solutions below

0
sim On

Your not working implementation misses the initialization of j. So it iterates the inner loop only in the first iteration of the outer loop.

void bubbleDesc (int n, int array[])
{
  
  int i = 0, j = 0, temp;
  
  for (i; i < n - 1; i++)
  {
    j = 0; // this init is needed
    for (j; j < n - 1; j++)
    {
      if (array[j] < array[j + 1])
      {
        temp = array[j + 1];
        array[j + 1] = array[j];
        array[j] = temp;
      }
    }
  }
  
}

Thought, better would be to limit the scope of j and declare it only in the block where it is used, i.e., for (int j=0;...){...}

0
Vlad from Moscow On

For starters it is better to specify in the function declaration the array as its first parameter and the number of elements in the array as its second parameter.

Also in C sizes of entities are defined as values of the type size_t.

So the function should be declared like

void bubbleDesc( int array[], size_t n );

In your first function implementation the variable j used in the inner for loop is initialized only once before the for loops

void bubbleDesc (int n, int array[])
{
  
  int i = 0, j = 0, temp;
  
  for (i; i < n - 1; i++)
  {
    for (j; j < n - 1; j++)
    //...

though you need to initialize it to 0 each time before the inner for loop will get the control. The second function implementation does such an initialization within the for loop statement.

void bubbleDesc (int n, int array[])
{
  int i, j, temp;
  
  for ( i  = 0; i < n - 1; i++ )
  {
    for ( j = 0; j < n - 1; j++ )
    //...

Bear in mind that you should declare variables in minimum scopes where they are used. That is it will be better at least to write

void bubbleDesc (int n, int array[])
{
  for ( int i = 0; i < n - 1; i++)
  {
    for (int j = 0; j < n - 1; j++)
    {
      if (array[j] < array[j + 1])
      {
        int temp = array[j + 1];
        array[j + 1] = array[j];
        array[j] = temp;
      }
    }
  }
  
}

Or if your compiler does not allow to declare variables in the for loop statement then you can write

void bubbleDesc (int n, int array[])
{
  int i = 0;
  for ( ; i < n - 1; i++)
  {
    int j = 0;
    for ( ; j < n - 1; j++)
    {
      if (array[j] < array[j + 1])
      {
        int temp = array[j + 1];
        array[j + 1] = array[j];
        array[j] = temp;
      }
    }
  }

But in any case your implementation of the function is not efficient because even when the whole array or its second part is already sorted nevertheless the loops continue its iterations for all elements of the array.

Another drawback is that if you will want to sort the array in the ascending order you will have to write a new function.

You should always try to write more general functions.

It will be better to add to the function a third parameter that will specify a comparison function that will compare elements of array.

Here is a demonstration program that shows how the function can be written.

#include <stdlib.h>

void bubbleSort( int a[], size_t n, int cmp( const void *, const void * ) )
{
    for (size_t last = 1; !( n < 2 ); n = last)
    {
        for (size_t j = last = 1; j < n; j++)
        {
            if (cmp( a + j, a + j - 1 ) < 0)
            {
                int tmp = a[j];
                a[j] = a[j - 1];
                a[j - 1] = tmp;
                last = j;
            }
        }
    }
}

static int ascending( const void *a, const void *b )
{
    int x = *( const int * )a;
    int y = *( const int * )b;

    return ( y < x ) - ( x < y );
}

static int descending( const void *a, const void *b )
{
    int x = *( const int * )a;
    int y = *( const int * )b;

    return ( x < y ) - ( y < x );
}

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

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

    bubbleSort( a, N, ascending );

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

    bubbleSort( a, N, descending );

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

The program output is

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