substituir
is unitialized. You need to set it to false
immediately after the for
statement for j
.
Your for
loop for j
is missing a trailing {
so it will only iterate over the first if
and not the others [as you would probably like]
As I mentioned in my comment, simplify [please ;-)]. Use pointers to simplify the code.
Your indexing is quite complex, so I can only guess at things.
I changed the comparison logic to something I understand.
Here's a simplified version. I just coded it, so it may not compile. But, it should give you some ideas how to proceed:
typedef struct Consulta {
char nomeUtente[70];
int numSNS;
int dia;
int mes;
int ano;
int horasInicio;
int minutosInicio;
int horasFim;
int minutosFim;
} consulta;
void
organizarAgenda(int membroEscolhido, consulta agenda[][50][50],
int clinicaSelecionada, int *nFuncionarios, int *nAgendas)
{
int i;
int j;
int lim = nAgendas[membroEscolhido];
int dif;
consulta temp;
for (i = 0; i < lim; i++) {
consulta *iptr = &agenda[i][membroEscolhido][clinicaSelecionada];
for (j = 0; j < lim; j++) {
consulta *jptr = &agenda[j][membroEscolhido][clinicaSelecionada];
do {
dif = iptr->ano - jptr->ano;
if (dif)
break;
dif = iptr->mes - jptr->mes;
if (dif)
break;
dif = iptr->dia - jptr->dia;
if (dif)
break;
} while (0);
if (dif <= 0)
continue;
temp = *iptr;
*iptr = *jptr;
*jptr = temp;
}
}
}
I'm [still] guessing but I think you can get a [significant] speedup by changing the for
loop for j
.
And, I think the for
loop for i
goes one too far.
So, consider:
for (i = 0; i < (lim - 1); i++) {
consulta *iptr = &agenda[i][membroEscolhido][clinicaSelecionada];
for (j = i + 1; j < lim; j++) {
consulta *jptr = &agenda[j][membroEscolhido][clinicaSelecionada];
UPDATE:
I didn't understand how a 3d array with only a 2d array assignment works int lim = nAgendas[membroEscolhido];
The value of nAgendas[membroEscolhido]
is invariant across the function, so it can be "cached". I did this to simplify the code [mostly] but it also can help the compiler generate more efficient code.
I didn't notice the (-) in the middle of this line, and the ->
works because it is a pointer pointing to the struct
, right?
Right. The arrow operator (->
) is a very powerful way to access individual struct
members if you have a pointer to the given struct instance.
Note that the compiler's optimizer might be able to see that all the variables of the form: array[x][y][z].whatever
could be reduced.
But, when we use intermediate pointers we're giving it a [better] clue as to what we want. And, the code is more readable by humans, so it has two good reasons to do it.
I don't understand why you put while (0)
This is a bit of a trick [of mine] to replace an if/else
ladder with something that is cleaner.
It forms a "once through" loop. It would be the equivalent of:
while (1) {
if (something)
break;
if (something_else)
break;
break; // make the loop execute _only_ once
}
For a more detailed explanation, see my answer: About the exclusiveness of the cases of an if block