Help With Bubble Sort

[eselar]

Hey everyone, I'm sorting an array randomly populated with the numbers 1-52 from least to greatest using multiple sorting algorithms,
right now I still cant get my bubble sort to behave

after i run the bubble sort, i print the elements; every element contains an assortment of characters like A0DDF288

There must be something wrong with my bubble sort

void BubbleSort()
{
int count=52;
bool moved;
do
{
moved=false;
for ( int i = 0; i < count; i++ )
{
if ( bubbleCards[i] > bubbleCards[i+1] )
{
int temp = bubbleCards[i];
bubbleCards[i] = bubbleCards[i+1];
bubbleCards[i+1] = temp;
bubbleSwap++;
moved=true;
}
bubbleComp++;
}
count--;
}
while (moved);
return;
}



FYI there are no parameters to this function because i simply defined it outside of main to look neat

any pointers would be helpful :)

[Chervil]

The definition of bubbleCards[] is not shown here.
However, based on the use of variable temp, int temp = bubbleCards[i]; it's reasonable to deduce that bubbleCards is an array of type int.

eselar wrote:

after i run the bubble sort, i print the elements; every element contains an assortment of characters like A0DDF288

Well, an integer variable cannot contain an assortment of characters. Most likely, there is something wrong with the code where you print the elements. Probably you are printing the address of the array rather than its contents.

But still, the sort does contain a serious error.
Look at this line: for ( int i = 0; i < count; i++ )

We see a little earlier that count is 52.
Thus i will vary from 0 to 51 (a total of 52 elements).

Now look at the next line:
if ( bubbleCards[i] > bubbleCards[i+1] )
When i is 51, bubbleCards[i] is the last item in the array.

So what then is bubbleCards[i+1] ?
It is the element after the last element, the 53rd element. Or to put it another way, it is outside the array, it is some other data that should not be accessed. Attempting to use this location can give rise to unpredictable results.

The solution? Change the for-loop to use count-1, like this:
for (int i = 0; i < count-1; i++ )



A more general comment, the function would look neater if it was clear what was its input and/or output. So, rather than declaring it like this, void BubbleSort(), it would be better to pass the array and its size as parameters, like this.

void BubbleSort(int bubbleCards[], int n)
{
    int count = n;
    bool moved;
    do
    {
        moved = false;
        for (int i = 0; i < count-1; i++ )
        {
            if ( bubbleCards[i] > bubbleCards[i+1] )
            {
                int temp         = bubbleCards[i];
                bubbleCards[i]   = bubbleCards[i+1];
                bubbleCards[i+1] = temp;
                moved = true;
            }
        }
        count--;
    }
    while (moved);
}

[3qX21hU5]

Would also just like to add-on to Chervil's great post. Like he said a more general way of the function is to declare parameters for the function. There are several reason for this.

1) You might want to reuse this code in some other project. Since you do not declare parameters and instead are using global variables it makes it so that this function can only be used with certain variable names. This makes it very hard for you and other people to re use this code.

2) Global variables can be very dangerous, and should not be used very often. My general rule is that if I can not use a global variable and still get the same result I wont use it. The reason they can be dangerous is that when you are working on bigger projects it is quite hard to keep track of things. So when I have a global variable it is available to the whole program to use and possibly change. It can be very time consuming trying to figure out why a certain output is not what it is suppose to be just because some line of code changed the global variable when I didn't want it to.

There are more reasons but them are the main two reasons I don't use them that often if at all.

[eselar]

Sorry i forget to mention some things.

yes it is an int array

I changed the heading of my for loop like Chervil said, which otherwise would have gave me incorrect results.

I review my code as i changed it and i noticed that the problem was a silly mistake, my loop that printed the elements was incorrect.

thank you for the advice of giving my functions parameters, i will be making those adjustments.

once again thank you for your time