• 02-21-2005
Apropos
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; }```
• 02-21-2005
sean
What is the actual problem with your output? What are you getting that you weren't expecting?
• 02-21-2005
Apropos
Quote:

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
• 02-21-2005
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
• 02-21-2005
Apropos
Quote:

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 :confused:
• 02-22-2005
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.
• 02-22-2005
pianorain
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]);```
• 02-22-2005
Brain Cell
- 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..
• 02-22-2005
Apropos
Hi Brain Cell;

Quote:

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.

Quote:

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. :rolleyes: )

Quote:

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.

Quote:

Originally Posted by Brain Cell
- Check this link for a much much easier way to sort your array.

The tutorial was great. Thanks
• 02-22-2005
Brain Cell
Quote:

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. :rolleyes: )

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.

Quote:

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?
• 02-22-2005
pianorain
Quote:

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. =/
• 02-22-2005
Brain Cell
Multiple declaration still cause confusion though :)