# C++: Number Arranging Program problem [closed]

I have tried making a program to sort numbers of an array.(Not sure if this is bubble-sort or not)

I have done my best but there is this problem: Although I do a loop to swap the numbers and arrange them, when I output the array, nothing changes and the array remains the same.

The code will make everything clearer

This is the main function:

int main(){
int arr[10];
//For loop to get from user numbers to be put into the array
for ( int i = 0; i<10; i++){
cout << "Enter the number to be recorded: ";
cin >> arr[i];
cout << endl;
}
// Set counter n to 0 ( counts numbes of number swaps)
int n = 0;
do {
//re sets counter to 0
n=0;

//Check the entire loop if arr[i] bigger than arr[i+1] and swaps their values if true then adds 1 to n
for ( int i = 0; i<9; i++){
if(arr[i]>arr[i+1]){
swap(&arr[i], &arr[i+1]);//swaps by sending the addresses of the two array elements the pointers in the swap function
n++;
}
}
}while(n>0); // if counter = 0 then end (therefore the numbers are arranged correctly since no swapping happened)
cout << "The numbers ordered are:\n\n";
// Loop to output the arranged array
for (int i =0; i<10; i++){
cout << arr[i] << ", ";
}
cout<<endl;
system("PAUSE");
return 0;}

This is the swap function:

void swap ( int *p, int *t){
int temp;
temp = *p;
*p = *t;
*t = temp;}

I hope you guys can help me with my problem here and tell me what's wrong with this code

Thank you all

-
You seem to have a type: for ( int i = 0; i>9; i++) Should that not be for ( int i = 0; i**<**9; i++) –  Loki Astari Jul 28 '12 at 0:27
Yes that is bubble sort. With the optimization of quitting if the list is already sorted. There is another optimization. The inner loop does not need to go all the way to 9 each time. After the first outer iteration finishes the largest number is at the top. So you don't need to re-test that again. After two outer iterations the top two values are sorted etc... –  Loki Astari Jul 28 '12 at 0:31
I would be more optimized if I did that. But it should at least work and sort. –  Mohamed Ahmed Nabil Jul 28 '12 at 0:32
BTW: C++ already has a std::swap(T&, T&) function, which you could use.(#include <algorithm>) –  luiscubal Jul 28 '12 at 1:15
People the program isnt re-ordering the elements of the array and i dont know why help –  Mohamed Ahmed Nabil Jul 28 '12 at 2:42

## closed as off topic by Winston EwertJul 29 '12 at 3:22

Questions on Code Review Stack Exchange are expected to relate to code review request within the scope defined by the community. Consider editing the question or leaving comments for improvement if you believe the question can be reworded to fit within the scope. Read more about reopening questions here.If this question can be reworded to fit the rules in the help center, please edit the question.

Your indenting style should remain consistent.

int main(){
int arr[10];

Here you have no indent after an open brace '{'

for ( int i = 0; i>9; i++){
if(arr[i]>arr[i+1]){

While here you indent by four.

Comments are a good thing. But you need to make them readable.

//Check the entire loop if arr[i] bigger than arr[i+1] and swaps their values if true then adds 1 to n

Try and chop them across multiple lines so people with smaller screen can still see them

// Check the entire loop if arr[i] bigger than arr[i+1]
// and swaps their values if true then adds 1 to n

When you write a comment your comment should explain why not what the code is doing. The code is always more precise then English so don't try and explain it just write good clean code. Your comments can then be reserved for explaining why you need to do something in a particular way.

For example:

// Set counter n to 0 ( counts numbes of number swaps)
int n = 0;

This is a terrible comment.
I can already see it is setting the value to zero so the comment is useless. The extra bit about the number of swaps tells me you have badly named your variable.

int numberOfSwaps = 0;

The problem with commenting the way you are doing it now is that your code can fall out of sync with the comments. This makes the comments not just bad but dangerous.

std::swap

void swap ( int *p, int *t){
int temp;
temp = *p;
*p = *t;
*t = temp;}

We already have a standard swap.

#include <algorithm>
template<typename T>
void swap(T& lhs, T& rhs);

Notice it is more convenient to use references than pointers for this type of thing.

The normal way of writing bubble sort is:

for (int i = 0; i < 9; ++i)
{
int numberOfSwaps = 0;

// Each time through the outer loop the largest element
// element is moved to the top. Thus we can reduce the
// size of the inner loop as we don't need to test it again.
for(int j = 0; j < (9 - i); ++j)
{
if(arr[j]>arr[j+1])
{
std::swap(arr[j], arr[j+1]);
++numberOfSwaps;
}
}
// If there was no swaps this time then the
// data is sorted. So we can escape the loop early.
if (numberOfSwaps == 0)
{   break;
}
}
-
Thank you alot for your help, I'm still new to c++ and learning. I have gotten to pointers so far and this program ( including the swap function) was a part of an exercise in the book im learning from. Also I still dont know templates or #include <algorithms> yet. I have gotten to pointers so far. (The book is C++ without fear) –  Mohamed Ahmed Nabil Jul 29 '12 at 0:13