c - Problem organizing a data set chronologically

Question

I'm here asking for your help, not because I have an error, but simply because of this solution that in my head seemed quite credible despite not working. I basically have a structure to make an appointment and I created a variable of this temporary structure to change the values ??so that they are in ascending order, however when I show the query table in this case, but the queries appear in the order I registered them in the program.

My Struct:

typedef struct Consulta {

    char nomeUtente[70];
    int numSNS;
    int dia;
    int mes;
    int ano;
    int horasInicio;
    int minutosInicio;
    int horasFim;
    int minutosFim;

} consulta;

My function that should order the values:

void organizarAgenda(int membroEscolhido, consulta agenda[][50][50], int clinicaSelecionada, int *nFuncionarios, int *nAgendas)
{
    
    int i, j;
    boolean substituir;
    consulta temp;

    
    for (i = 0; i < nAgendas[membroEscolhido]; i++)
    {
        for (j = 0; j < nAgendas[membroEscolhido]; j++)
    
            if (agenda[j][membroEscolhido][clinicaSelecionada].ano > agenda[i][membroEscolhido][clinicaSelecionada].ano)
                substituir = true;
                
            if (agenda[j][membroEscolhido][clinicaSelecionada].ano == agenda[i][membroEscolhido][clinicaSelecionada].ano 
            && (agenda[j][membroEscolhido][clinicaSelecionada].mes > agenda[i][membroEscolhido][clinicaSelecionada].mes))
                substituir = true;
                
            if (agenda[j][membroEscolhido][clinicaSelecionada].ano == agenda[i][membroEscolhido][clinicaSelecionada].ano 
            && (agenda[j][membroEscolhido][clinicaSelecionada].mes == agenda[i][membroEscolhido][clinicaSelecionada].mes)
            && (agenda[j][membroEscolhido][clinicaSelecionada].dia > agenda[i][membroEscolhido][clinicaSelecionada].dia))
                substituir = true;
                
            if (agenda[j][membroEscolhido][clinicaSelecionada].ano == agenda[i][membroEscolhido][clinicaSelecionada].ano 
            && (agenda[j][membroEscolhido][clinicaSelecionada].mes == agenda[i][membroEscolhido][clinicaSelecionada].mes)
            && (agenda[j][membroEscolhido][clinicaSelecionada].dia == agenda[i][membroEscolhido][clinicaSelecionada].dia
            && agenda[j][membroEscolhido][clinicaSelecionada].horasInicio >= agenda[i][membroEscolhido][clinicaSelecionada].horasInicio))
                substituir = true;
        
        
            if (substituir == true) 
            {
                
                //Igualar a variavel temporario á variável agenda em i
                temp.ano = agenda[i][membroEscolhido][clinicaSelecionada].ano;
                temp.mes = agenda[i][membroEscolhido][clinicaSelecionada].mes;
                temp.dia = agenda[i][membroEscolhido][clinicaSelecionada].dia;
                temp.horasInicio = agenda[i][membroEscolhido][clinicaSelecionada].horasInicio;
                temp.minutosInicio = agenda[i][membroEscolhido][clinicaSelecionada].minutosInicio;
                temp.horasFim = agenda[i][membroEscolhido][clinicaSelecionada].horasFim;
                temp.minutosFim = agenda[i][membroEscolhido][clinicaSelecionada].minutosFim;

                //Igualar a variável agenda em i á variável agenda em j
                agenda[i][membroEscolhido][clinicaSelecionada].ano = agenda[j][membroEscolhido][clinicaSelecionada].ano;
                agenda[i][membroEscolhido][clinicaSelecionada].mes = agenda[j][membroEscolhido][clinicaSelecionada].mes;
                agenda[i][membroEscolhido][clinicaSelecionada].dia = agenda[j][membroEscolhido][clinicaSelecionada].dia;
                agenda[i][membroEscolhido][clinicaSelecionada].horasInicio = agenda[j][membroEscolhido][clinicaSelecionada].horasInicio;
                agenda[i][membroEscolhido][clinicaSelecionada].minutosInicio = agenda[j][membroEscolhido][clinicaSelecionada].minutosInicio;
                agenda[i][membroEscolhido][clinicaSelecionada].horasFim = agenda[j][membroEscolhido][clinicaSelecionada].horasFim;
                agenda[i][membroEscolhido][clinicaSelecionada].minutosFim = agenda[j][membroEscolhido][clinicaSelecionada].minutosFim;
                
                //Igualar a variável agenda em j á variavel temporaria
                agenda[j][membroEscolhido][clinicaSelecionada].ano = temp.ano;
                agenda[j][membroEscolhido][clinicaSelecionada].mes = temp.mes;
                agenda[j][membroEscolhido][clinicaSelecionada].dia = temp.dia;
                agenda[j][membroEscolhido][clinicaSelecionada].horasInicio = temp.horasInicio;
                agenda[j][membroEscolhido][clinicaSelecionada].minutosInicio = temp.minutosInicio;
                agenda[j][membroEscolhido][clinicaSelecionada].horasFim = temp.horasFim;
                agenda[j][membroEscolhido][clinicaSelecionada].minutosFim = temp.minutosFim;
                
            }

    }

Thank you all in advance!

Answer 1

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