CODEWITHSUNDEEP
c#
Arianule
Arianule

Reputation: 9053

bubble sort logic error

I was trying a basic sorting exercise and I was hoping I could receive some help with what is probably a basic logic error.

int[] numbers = new int[] { 2, 5, 11, 38, 24, 6, 9, 0, 83, 7 };
        for (int loop = 0; loop < numbers.Length; loop++)
        {
            Console.WriteLine(numbers[loop]);
        }
        Console.WriteLine("Performing a bubble sort");

        bool flag = false;
        do
        {

            for (int loop = 0; loop < numbers.Length - 1; loop++)
            {
                if (numbers[loop] > numbers[loop + 1])
                {

                    int temporary = numbers[loop];
                    numbers[loop] = numbers[loop + 1];
                    numbers[loop + 1] = temporary;
                    flag = true;
                }

            }
        } while (flag == false);

        for (int loop = 0; loop < numbers.Length; loop++)
        {
            Console.WriteLine(numbers[loop]);
        }

Upvotes: 0

Views: 406

Answers (6)

Karel Frajt&#225;k
Karel Frajt&#225;k

Reputation: 4489

Bubble sort is not one pass sorting. In one iteration the biggest number moves to the most right cell. So after the first iteration the largest number will be stored in the last cell.

for (int l = numbers.Length - 1; l > -1; l--)  
  for (int loop = 0; loop < l; loop++) { /* your code */ }

And you don't need the flag (ok, maybe you need, but that not the mistake you've made)

Here's the output of the algorithm after each iteration:

2 5 11 24 6 9 0 38 7 83
2 5 11 6 9 0 24 7 38 83
2 5 6 9 0 11 7 24 38 83
2 5 6 0 9 7 11 24 38 83
2 5 0 6 7 9 11 24 38 83
2 0 5 6 7 9 11 24 38 83
0 2 5 6 7 9 11 24 38 83
0 2 5 6 7 9 11 24 38 83
0 2 5 6 7 9 11 24 38 83
0 2 5 6 7 9 11 24 38 83

Upvotes: 0

dlev
dlev

Reputation: 48596

There are two issues with your code. The first, as has been pointed out, is that you need to loop as long as flag == true. That would have been a lot more clear if you had given it a more expressive name. madeASwap or something like that makes it obvious: do while(madeASwap).

The other issue is that you need to reset the flag before running the inner loop. Without that, just checking for false ends after a single iteration, and checking for true results in an infinite loop.

In short: reset your flag, and loop while it's true.

Upvotes: 1

John
John

Reputation: 6668

Your loop condition while(flag == false) should read while(flag == true)

Upvotes: 0

James Michael Hare
James Michael Hare

Reputation: 38427

I don't know everything that's wrong, but one thing for sure is that your do/while loop should be going while while(flag == true), not while(flag == false). Which, of course, can be written more simply as while(flag)

Upvotes: 2

Jimmy
Jimmy

Reputation: 91482

Your flag logic is wrong. Everything else looks correct.

The flag is supposed to mean:

loop until we looped without making any swaps

But that isn't what your code currently does.

Upvotes: 1

Royi Namir
Royi Namir

Reputation: 148644

take a look here and migrate : 

// array of integers to hold values
private int[] a = new int[100];

// number of elements in array
private int x;

// Bubble Sort Algorithm
public void sortArray()
{
  int i;
  int j;
  int temp;

  for( i = (x - 1); i >= 0; i-- )
  {
    for( j = 1; j <= i; j++ )
    {
      if( a[j-1] > a[j] )
      {
        temp = a[j-1];
        a[j-1] = a[j];
        a[j] = temp;
      }
    }
  }
}

Upvotes: 0

Related Questions