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;
}

---

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

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

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

Quote:

---

Originally Posted by elad

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:

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;

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

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?

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

Multiple declaration still cause confusion though :)