1. ## help about bubble sort

Hi@all

I'm learning the bubble sort from my textbook. I just wrote a small program but something is wrong about output. I couldn't see my mistake. Please help me to see what is wrong with it.

Thanks

Code:
```#include <iostream>
using namespace std;

const int size=5;

int main()
{
int m=0;
int array[size];
int counter=0;

cout <<"Enter some integers "<<endl;

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

for(int m=0; m<counter-1; m++)
{
int holder=0;

for (int i=0; i<counter-1; i++)
{

int j=i;

if (array[i]>array[j++])
{

holder=array[j];

array[i]=array[j];

array[i]=holder;
}
}
}

for (int i=0; i<counter; i++)
cout <<array[i]<<" ";

return 0;
}```

2. What is the actual problem with your output? What are you getting that you weren't expecting?

3. Originally Posted by sean_mackrory
What is the actual problem with your output? What are you getting that you weren't expecting?
Assume I typed 5,6 and 1 as my input. My output should look like 1 5 6 but I get the same as what I typed.

Thanks

4. 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

change j++ to ++j
I think you are right. I changed j++ to ++j but still something is wrong because when I write 5 4 3 2 1, the output is 1 1 1 1. I am

6. 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.

7. 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]);```

8. - 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..

9. 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

10. 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?

11. 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. =/

12. Multiple declaration still cause confusion though