# Thread: Bubble sort isnt completely working

1. ## Bubble sort isnt completely working

so i basically looked at some pseudo code and tried to code it but when i run it i get like one number out of order, any idea?

Code:
```#include <iostream>

using namespace std;

#define NUMBER_OF_ELEMENTS 15

int main(){
int toSort[NUMBER_OF_ELEMENTS];

cout<< "Enter the numbers:\n ";
for (int i = 0; i < NUMBER_OF_ELEMENTS; i++){
cout<< "\n\t";
toSort[i] = rand();
}

bool sorting = true;
do{
int total = 0;
for(int b = 0; b <= NUMBER_OF_ELEMENTS; b++){
if(toSort[b + 1] < toSort[b]){
int tmp = toSort[b];
toSort[b] = toSort[b + 1];
toSort[b + 1] = tmp;
}
else{
total++;
}
}
if (total >= NUMBER_OF_ELEMENTS){
sorting = false;
}
} while(sorting == true);

cout<<"Everything sorted:";

for (int z = 0; z < NUMBER_OF_ELEMENTS; z++){
cout<<"\n\t";
cout<<toSort[z];
}

}```

2. At a glance, this line looks suspect:
Code:
`for(int b = 0; b <= NUMBER_OF_ELEMENTS; b++){`
In the loop, you access toSort[b + 1]. This means that if b == NUMBER_OF_ELEMENTS, you are accessing toSort out of bounds, since you will be accessing toSort[NUMBER_OF_ELEMENTS + 1], when in fact the last element of toSort that you may access is toSort[NUMBER_OF_ELEMENTS - 1].

3. Except for the issues laserlight pointed out, this...
Code:
```                int tmp = toSort[b];
toSort[b] = toSort[b + 1];
toSort[b + 1] = tmp;```
...is equal to...
Code:
`std::swap(toSort[b], toSort[b + 1]);`
Just pointing that out.

4. Worse still, you've over-running the array by TWO items!
Subtract one AND use strictly less-than in your loop condition.

I'm surprised you came up with the idea of counting the non-swaps rather than the simpler option of just setting a flag for the actual swaps.

5. Okay guys thanks for the input, I went back and edited the code, and I actually meant to ask about a function to swap memebers of any array in the op. Anyway this works, and I changed it to where it doesn't have a counter and makes sorting true again, so it will keep looping, but if i remember correctly my execution time was less when i had the counter.

Code:
```#include <iostream>

using namespace std;

#define NUMBER_OF_ELEMENTS 200

int main(){
int toSort[NUMBER_OF_ELEMENTS];

for (int i = 0; i < NUMBER_OF_ELEMENTS; i++){
toSort[i] = rand();
}

bool sorting = true;
do{
sorting = false;
int total = 0;
for(int b = 1; b < NUMBER_OF_ELEMENTS; b++){
if(toSort[b - 1] > toSort[b]){
swap(toSort[b], toSort[b - 1]);
sorting = true;
}
}
} while(sorting == true);

cout<<"Everything sorted:";

for (int z = 0; z < NUMBER_OF_ELEMENTS; z++){
cout<<"\n\t";
cout<<toSort[z];
}

}```

6. You can remove the unused variable 'total'. Otherwise that looks good now.

7. You should #include <algorithm> for std::swap instead of relying on it being indirectly included. Personally, I would change while (sorting == true) to while (sorting), since it is clear that sorting is a boolean flag variable.