Thread: help about bubble sort

I think your problem is in these two lines:

int j=i;
if (array[i]>array[j++])

j++ means return j and then increment j by 1. Thus the conditional essentially measures array[i] against array[i] which will never cause the conditional to be true so no swapping will occur. change j++ to ++j or drop j altogether and use i + 1 in place of j++, and make appropriate adjustment in the swap section of the code

holder=array[j];
array[i]=array[j];
array[i]=holder;

Why do you assign both array[j] and holder to array[i]? Work out the appropriate logic, and I think it will work better.

As elad said, your swap is a little broken. You may find it easier to use the STL swap algorithm, since it avoids little mistakes like this.

Code:

#include <algorithm>
...
/*
    holder=array[j];
    array[i]=array[j];
    array[i]=holder;
*/
    swap(array[i],array[j]);

- there's no reason to make size global.

- you declared m twice.

- you used :

Code:

while (!cin.eof())
        {
                m++;
                counter++;
                cin>>array[m];
        }

to input values when you can simply use a 'for' loop to do the same job in less code.

- Check this link for a much much easier way to sort your array.


Hope this helps..

Hi Brain Cell;

Originally Posted by Brain Cell

-there's no reason to make size global.

Yes you are right. I think because of the eof. I don't need size. I use a counter to find out the size.

Originally Posted by Brain Cell

- you declared m twice.

I am not agree about it because one m is inside the for loop. I also didn't get any error because of it from g++. (I just started to learn c++ so I am not sure. )

Originally Posted by Brain Cell

to input values when you can simply use a 'for' loop to do the same job in less code.

I was reading from my book. It was saying that for loop is good when you know the size but here I don't know the capacity of my array. I guess you were cofiused because of const. It was my fault sorry.

Originally Posted by Brain Cell

- Check this link for a much much easier way to sort your array.

The tutorial was great. Thanks

Originally Posted by Apropos

I am not agree about it because one m is inside the for loop. I also didn't get any error because of it from g++. (I just started to learn c++ so I am not sure. )

I don't know about g++, but i use Visual C++ 6 and it won't let me compile it because of "error C2374: 'm' : redefinition; multiple initialization". It should be declared only once.

Originally Posted by Apropos

I was reading from my book. It was saying that for loop is good when you know the size but here I don't know the capacity of my array.

What do you mean you don't know the capacity of your array? you declared your array as :

Code:

int array[size];

size equal 5 wich means your array can hold 5 integer values. You can simply replace that part of your code with :

Code:

for(m=0; m<size; m++)
   cin >> array[m];

see how easy it is now?

Originally Posted by Brain Cell

I don't know about g++, but i use Visual C++ 6 and it won't let me compile it because of "error C2374: 'm' : redefinition; multiple initialization". It should be declared only once.

This is due to Microsoft's language extensions. You can disable them by going to your Project Settings under the C/C++ tab and selecting the Customize option. There you can check a box to disable language extensions. Don't expect the Microsoft implementation of STL to compile without them, though. =/

Multiple declaration still cause confusion though