ordering algorithm

I'm writing an ordering algorithm for a defined 10-elements array but it doesn't work. The array is always the same.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
#include<stdio.h>

int v[10]={5,4,2,6,8,9,1,0,3,7},i;

//Funzione per lo scambio
void swap(int *a,int *b){
	int *tmp;
	tmp=a;
	a=b;
	b=tmp;
}

//Algoritmo
int algo(int *v){
	int max, n=10;
	for(i=0; i<10; i++){
		if(v[i]>v[i+1]){
			max=v[i];
		}
		swap(&v[max], &v[n-1]);
		n--;
	}
}

//Corpo del programma
int main(){
	algo(v);
	printf("Vettore ordinato:%d",v[0]);
	for(i=1; i<10; i++){
		printf(",%d",v[i]);
	}
}
Your swap function does nothing. You are swapping (local) pointers. YOu need to swap values instead.
The first thing I notice is that your swap function does not work.

you change the value of the non-dereferenced pointer, not the dereferenced one.

try it like this
1
2
3
4
5
6
void swap(int *a,int *b){
    int tmp;
    tmp = *a;
    *a = *b;
    *b = tmp;
}


... or in C++03 with references
1
2
3
4
5
6
void swap(int& a,int& b){
    int tmp;
    tmp = a;
    a = b; 
    b = tmp;
}


... and with templates
1
2
3
4
5
6
7
template <typename T>
void swap(T& a, T& b){
    T tmp;
    tmp = a;
    a = b;
    b = tmp;
}


... and using the copy constructor
1
2
3
4
5
6
template <typename T>
void swap(T& a, T& b){ 
    T tmp = a; // copy construction
    a = b;
    b = tmp;
}


... and in C++11 with move
1
2
3
4
5
6
template <typename T>
void swap(T& a, T& b){
    T tmp = std::move(a);
    a = std::move(b);
    b = std::move(tmp);
}


The last version is basically the std::swap function
You can go with any of those, use one you understand or learn how the others work, it's up to you :)
Last edited on
I just added a function for verifying if the array is ordered but it doesn't print the array.

#include<stdio.h>

int v[10]={5,4,2,6,8,9,1,0,3,7},i,prod=1;

//Funzione per lo scambio
void swap(int *a,int *b){
int tmp;
tmp=*a;
*a=*b;
*b=tmp;
}

//Funzione per la verifica del vettore
int order(int *v){
for(i=0; i<10; i++){
if(v[i]<v[i+1]){
prod=prod*1;
}
else{
prod=prod*0;
}
}
return prod;
}

//Algoritmo
int algo(int *v){
for(i=0; i<10; i++){
if(v[i]>v[i+1] && i!=9){
swap(&v[i], &v[i+1]);
}
}
}

//Corpo del programma
int main(){
do{
algo(v);
}while(order(v)==0);
printf("Vettore ordinato:%d",v[0]);
for(i=1; i<10; i++){
printf(",%d",v[i]);
}
}

Is there any reason for using printf? I think the cout looks nicer?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
#include <iostream>

using namespace std;

int main()
{
    int v[10]={5,4,2,6,8,9,1,0,3,7};

    cout << "sorted array: ";

    for(int i=0; i<10; i++)
    {
        cout << v[i] << " ";
    }
    cout << endl;
    return 0;
}

i didn't study it yet!! Did you find a mistake in my algorithm?
One way to find mistakes is to use print out data.
I used "GOT HERE" to show it never got there.
And then placed a print out in the algo routine and it becomes apparent it is caught in a loop.
So now you step through the loop to see why.
It is kind of fun like being a detective and very satisfying when you figure it out although probably not as satisfying as having it run perfect first time :)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
#include<stdio.h>

int v[10]={5,4,2,6,8,9,1,0,3,7};
int i = 0;
int prod=1;

//Funzione per lo scambio
void swap(int *a,int *b){
    int tmp;
    tmp=*a;
    *a=*b;
    *b=tmp;
}

//Funzione per la verifica del vettore
int order(int *v)
{
    for(int i=0; i<10; i++){
    if(v[i]<v[i+1]){
        prod=prod*1;
        }
    else{
        prod=prod*0;
        }
    }
return prod;
}

//Algoritmo
void algo(int *v)
{
    for(i=0; i<10; i++){
        if(v[i]>v[i+1] && i!=9){
            swap(&v[i], &v[i+1]);
        }
        printf(",%d",v[i]);
    }
}

//Corpo del programma
int main()
{
    // print array
    for(int i=0; i<10; i++){
        printf(",%d",v[i]);
    }

    do{
        algo(v);
    }while(order(v)==0);

    printf("GOT HERE /n");

    printf("Vettore ordinato:%d",v[0]);

    for(int i=0; i<10; i++){
        printf(",%d",v[i]);
    }
}

Last edited on
The most glaring mistake is here:
1
2
3
4
5
6
7
8
9
10
11
12
int order(int *v)
{
    for(int i=0; i<10; i++){
    if(v[i]<v[i+1]){
        prod=prod*1;
        }
    else{
        prod=prod*0;
        }
    }
return prod;
}
First of all using int to return success/failure is a C way. You are not actually using C++. In fact your whole code is in C.

Ok, now to the actual problems.
If prod is 0 when you called your function, it will stay that way. Nothing changes it to 1. this stems from next problem:
Global prod variable. If you have global variable, you should stop and reconcider "should I have it at all? How can I get rid of it?"
In your case that variable brings state to order function. Now it depends on result of previous execution.
So your function is better be named "was_unsorted_array_passed_at_any_time" right now.
MiiNiPaa,
Most glaring mistake to an experienced c++ programmer :)
Beginners are lucky to have someone like you to help them out.
My interest was as someone learning c++ to see if I can figure out the problems to see what I still have to learn.
It became obvious to me that
while(order(v)==0);
was always true and thus went on forever
At a glance the purpose of order(), that is, the algorithm isn't clear to me.

Last edited on
:-)
Topic archived. No new replies allowed.