Qsort ordering in the wrong way

Asked

Viewed 113 times

1

My activity is to read a sentence and ordered in alphabetical order, but is giving error

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int ordena(const void * a, const void * b)
{
   return strcmp((char *)a, (char *)b); 
}

int main(int argc, char** argv)
{
  char **nomes, frase[900], *ponteiro;
  nomes = malloc(sizeof(char*) * 3000);
  int indice, cont = 0, i;
  for(indice = 0; indice < 3000; indice++)
  {
    nomes[indice] = malloc(sizeof(char) * 100);
  }

  scanf("%[^\n]", frase);

  ponteiro = strtok(frase, " ");
  while(ponteiro != NULL)
  {
     strcpy(nomes[cont], ponteiro);
     cont++;
     ponteiro = strtok(NULL, " ");
  }
  qsort((void*)nomes, cont, sizeof(nomes[0]), ordena);
  for(i = 0; i < cont; i++)
  {
    printf("%s\n", nomes[i]);
  }

   return 0;
}

Example of orange meat starter juice orange pickles orange pickles

Example of orange beef output pickles pickles juice

  • 1

    For me, your program generates as output picles picles laranja laranja suco carne. I couldn’t find where the mistake was.

2 answers

4


Your problem is actually quite typical in the qsort, which is to forget that the comparison function receives pointers to the array values. So if you have an array of ints the comparison function receives two int*. In your case you have an array of char* the function will receive two char**, what makes this:

int ordena(const void * a, const void * b)
{
   return strcmp((char *)a, (char *)b);
                     ^----------^ 
}

Is not correct.

Instead you have to interpret each parameter as one char** and access the target value char* who intends:

int ordena(const void * a, const void * b)
{
    char **palavra1 = (char**) a; //interpretar como char**
    char **palavra2 = (char**) b; //interpretar como char**
    return strcmp(*palavra1, *palavra2); //comparar o apontado de cada um
}

With this change the output gives:

carne
laranja
laranja
picles
picles
suco

View execution in Ideone

As a note, the cast for void* in the qsort is not necessary, ending up being simpler and readable without it:

qsort((void*)nomes, cont, sizeof(nomes[0]), ordena);
        ^---este
  • Why strcmp(*palavra1, *palavra2) and not strcmp(palavra1, palavra2)?

  • 1

    @Marcelouchimura Because in my comparison function palavra1 is a char** and not a char*, then it is necessary to seek value pointed with *palavra1. But this is no different than what you did in your code with *(char**)item1 in contrast to (char**)palavra1. If you look closely you’ll notice that yours has the * What’s more, that in mine was the call to strcmp.

  • 1

    @Marcelouchimura As an addendum to what I have already said, I do not see how your answer is different from mine, with the aggravating of not even explaining what you did or did not do, leaving a mere "See if this solves..."

  • I offered a complete answer. (my) program runs, as you might expect it to do.

  • 3

    @Marcelouchimura I must remind you that the goal of Stackoverflow is not to promote the copy answer code blind. The first relevant thing is that the author of the question understand what he did wrong and why he was wrong, so that he could understand the solution and implement it. Offering a code-only answer is not what we want to do on Sopt, and that’s basically what you did. If I ask a question and they give me two pages of code with "See if this solves" as I know what I did wrong to avoid doing it again ?

  • 2

    @Marcelouchimura But don’t be influenced by my words, open any reply with good punctuation in this network and see if they have only code with a "See if this solves".

  • It is part of the learning process of a Coder know how to read code. A third-rate employer wouldn’t mind using this and other codes to test candidates.

  • 1

    Thank you very much Isac

  • 1

    Very also Marcelo

  • 3

    @Marcelouchimura You are purposely misrepresenting what is being said. The purpose of an answer in Sopt is not to force the author of the question to learn how to read code written by others, nor to prepare candidates for the labour market. The idea is to provide content on programming issues that is direct, accessible and perceptive to as many people as possible.

  • So Stackoverflow would be like the rare patient’s emergency room? Or the terminal patient, which is more virtuous?

  • 4

    @Marcelouchimura This question is totally out of touch with reality. You have many questions in this network and in the English network of the OS only theoretical level about certain programming concepts. In these cases it is visible that the author only seeks to clarify the concept or deepen his knowledge in it. Regardless, I don’t even see how what I said relates to what I said earlier. But nothing better than an example.Tell me if this issue is an emergency room?

  • @Isac, it’s an ER of ramblings. (A) Questioner(a) may reflect and find the conclusion on its own, but how do you think you can’t, or how do you think you can’t whether, which is most likely, flaunts the question in the forum for others to conclude by it(a).

  • @Isac, I’m sorry if my pragmatism hits you, but it’s my style. I’m not here for shows, but to solve problems.

  • 3

    @Marcelouchimura Amigo nothing you’re saying "hits" me. I’m just in good faith trying to show you how the Sopt community works, something I’ve come to realize with all the time that I already have active participation in it. But it seems to me that you do not care, and so I quickly end our debate here.

Show 10 more comments

0

Pay close attention to RAM memory misalignment after use! free in them!

See if this resolves:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

char** palavras;

int comparador(const void* item1, const void* item2)
{
    char* elemento1 = *(char**)item1;
    char* elemento2 = *(char**)item2;

    return strcmp(elemento1, elemento2);
}

int main()
{
    char palavra[80];
    int qtdPalavras;
    int tamanhoPalavra;
    int i;

    do
    {
        palavras = NULL;

        fflush(stdin);
        printf("Digite a frase (8 para sair): ");

        qtdPalavras = 0;
        do {
            scanf("%s", palavra);
            tamanhoPalavra = strlen(palavra);
            if (*palavra != '8')
            {
                palavras = (char**)realloc(palavras, (qtdPalavras + 1) * sizeof(char*));
                palavras[qtdPalavras] = (char*)malloc((tamanhoPalavra + 1) * sizeof(char));
                strcpy(palavras[qtdPalavras], palavra);
                ++qtdPalavras;
            }
        } while (*palavra != '8');

        if (qtdPalavras)
        {
            printf("Ordenando a lista de palavras...\n");
            qsort(palavras, qtdPalavras, sizeof(char*), comparador);
            printf("Lista ordenada:\n");
            for (i = 0; i < qtdPalavras; ++i)
            {
                printf("%s ", palavras[i]);
            }
            printf("\n");
        }

        for (i = 0; i < qtdPalavras; ++i)
        {
            free(palavras[i]);
        }

        free(palavras);
    } while (qtdPalavras);

    return 0;
}
  • 5

    Although I find it commendable to promote the use of free, you have turned the question author’s problem into something completely different without giving a reasonable justification for doing so. The problem was to read a user phrase and sort the words, but you filled in things that had nothing to do with the question by asking the user to type one word at a time and complicate the problem with complex logic of memory allocation and reallocation. Besides, he doesn’t accept words like x-salada, xenônio, xerox or xícara.

  • @Victorstafusa, I will change the program then to accept such words.

  • @Victorstafusa, allocating memory is no easy task at all; that is why, in other programming languages, there are garbage collectors and automatic memory management.

  • Another detail that needs to be said is that the program shown accepts any number of words, as long as they have a maximum of 80 characters.

  • And no need to enter "one word at a time". An entrance of the kind casa banheiro sala cozinha quarto despensa garagem, words separated by spaces, is completely acceptable. Just make sure that the end of the sentence is marked with an escape character, which now instead of X, is the 8.

  • @Victorstafusa, how about offering an example like 8-salada, 8enonio, 8erox or 8icara now? Try everything in one sentence, 8-salada 8enonio 8erox 8icara 8. If you have any further questions, let me know.

Show 1 more comment

Browser other questions tagged

You are not signed in. Login or sign up in order to post.