Qsort array of strings in alphabetical order

Question

Im trying to sort a array of strings I read from a file in alphabetical order using the qsort function. This is my code:

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

#define MAXCHAR 256


int main(int argc, char **argv){
    char tempList[MAXCHAR][MAXCHAR];
    char reader[MAXCHAR];
    FILE* fp;
    int i;
    char *n;
    int index = 0;
    if(argc != 2){
        printf("too few arguments\n");
        exit(-1);
    }

    fp=fopen(argv[1], "r");
    if(fp == NULL){
        printf("failed to open file\n");
        exit(-1);
    }
    while(!feof(fp)){
        fgets(reader, MAXCHAR, fp);
        n = strchr(reader, '\n');
        if(n != NULL){
            *n = '\0';
        }
        strcpy(tempList[index], reader);
        index++;
    }
    index--;
    for(i=0; i<index; i++){
        printf("%s\n", tempList[i]);

    }
    qsort(tempList, index, sizeof(char *), strcmp);

    for(i=0; i<index; i++){
        printf("%s\n", tempList[i]);
    }
}

When I run the program, the list doesn't get sorted at all. I also tried methods posted on this website that asks a similar question and they all give me segmentation faults. Is there something wrong with the code?

Here is part of the content of the txt file. It is a list of 40 names:

Liam
Alexander
Mia
Noah
Emma
William
Charlotte
Charlotte
Mason
William
Ethan
Ethan
Liam
Alexander
Liam
Sophia
Emily
Mason
Alexander

Probably unrelated to the problem, but..Why is “while ( !feof (file) )” always wrong? — Eugene Sh.
– Eugene Sh., Commented Jul 31, 2015 at 17:24
Instead of the feof I suggest while(fgets(reader, MAXCHAR, fp) != NULL) — Weather Vane
– Weather Vane, Commented Jul 31, 2015 at 17:36
Also using MAXCHAR for the name length space and for the number of names is very poor practice - can lead to confusion when using MAXCHAR. — Weather Vane
– Weather Vane, Commented Jul 31, 2015 at 18:32

WhozCraig · Accepted Answer · 2015-07-31 18:07:50Z

4

You have a bona fide array of arrays; not an array of char*. Arrays aren't pointers. qsort expects the stride of the elements in the sequence being sorted. As your sequence is declared as:

char tempList[MAXCHAR][MAXCHAR];

the proper element size is the size of the inferior element size. In this case you have an array of size MAXCHAR of array of char of size MAXCHAR (an array of arrays).

Thus this is wrong:

qsort(tempList, index, sizeof(char *), strcmp);
// wrong size ==============^^^^

the size of each element should be:

qsort(tempList, index, sizeof tempList[0], strcmp);
// correct size ==============^^^^

The other issues in your program will eventually grief you and are covered in general comments below your question, but this is the fundamental problem preventing your sorting from working correctly. A retooled version of your program appears below, addressing most of those concerns:

Updated Source

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

#define MAXCHAR 256

/* properly declared for compatibility with qsort */
static int cmp_str(const void *lhs, const void *rhs)
{
    return strcmp(lhs, rhs);
}

/* main entrypoint */
int main(int argc, char **argv)
{
    char tempList[MAXCHAR][MAXCHAR];
    FILE* fp;
    size_t i, index = 0;

    if(argc != 2)
    {
        printf("too few arguments\n");
        return EXIT_FAILURE;
    }

    fp=fopen(argv[1], "r");
    if(fp == NULL)
    {
        perror(argv[1]);
        return EXIT_FAILURE;
    }

    while(index < MAXCHAR && fgets(tempList[index], sizeof(*tempList), fp) != NULL)
    {
        char *n = strchr(tempList[index], '\n');
        if(n != NULL)
            *n = 0;
        if (*(tempList[index])) /* don't insert blank lines */
            ++index;
    }

    for(i=0; i<index; i++)
        printf("%s\n", tempList[i]);
    fputc('\n', stdout);


    qsort(tempList, index, sizeof tempList[0], cmp_str);

    for(i=0; i<index; i++)
        printf("%s\n", tempList[i]);

    return EXIT_SUCCESS;
}

Untested, but it should be pretty close.

Best of luck.

answered Jul 31, 2015 at 18:07

WhozCraig

66.5k11 gold badges82 silver badges148 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

NewbieProgrammer Over a year ago

thanks! it works! quick questions though: 1) I heard its a bad practice to not brace after a if(), for(), while(), etc. statements but it seems like you (and alot of other programmers) still use it. was i misinformed when i heard its a bad practice? 2) is EXIT_FAILURE and EXIT_SUCCESS the same thing as exit(-1), exit(0)? are the keywords part of <stdlib.h>? 3) dont i require <io.h> (<unistd.h> in mac i believe) for file reading and writing? 4) is size_t another way of writing int?

WhozCraig Over a year ago

1. Extremely opinionated question that that has pros and cons in both camps. But you should notice proper spacing and indentation are also important. 2. They're defined by the standard to be proper for your platform. That is why I use them. 3. This program uses all standard-library functions. no need for unix or mac-specific headers. 4. size_t is likewise defined by the standard as an unsigned type compatible with sizeof, etc. Read here to learn more about it and its typical usage.

Robert Jacobs · Accepted Answer · 2015-07-31 18:26:52Z

2

Your size value in qsort(tempList, index, sizeof(char *), strcmp); is wrong. It should be qsort(tempList, index, sizeof(*tempList), strcmp);.

edited Jul 31, 2015 at 18:26

answered Jul 31, 2015 at 18:04

Robert Jacobs

3,3701 gold badge22 silver badges31 bronze badges

2 Comments

Weather Vane Over a year ago

In this case, the number of strings is the same as the string length, MAXCHAR. Better to do as @itsnotmyrealname answered and pass sizeof(*tempList) to qsort. Then if you change one or the other array size, this will spare any confusion.

Weather Vane Over a year ago

As you have edited the answer, I'll point out that sizeof(*tempList) is the same as sizeof tempList[0] as posted by @WhozCraig.

Soner from The Ottoman Empire · Accepted Answer · 2015-07-31 18:09:03Z

1

I have tried to fix your code.

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
//#include<io.h>    it's not standart


#define MAXCHAR 256

//  I implement the function because the warning is that
//  Incompatible pointer types passing 'int
//  (const char *, const char *)' to parameter of type
//  'int (*)(const void *, const void *)'

//  Already qsort() prototype is

// void qsort(void* ptr, size_t count, size_t size,
// int (*comp)(const void*, const void*));

// I think that the warning can be ignored strcmp also can be used

int myCompare(const void* a, const void* b)
{
    const char* aa = (const char*)a;
    const char* bb = (const char*)b;
    return strcmp(aa, bb);
}

int main(int argc, char **argv){
    char tempList[MAXCHAR][MAXCHAR];
    char reader[MAXCHAR];
    FILE* fp;
    int i;
    char *n;
    int index = 0;
    if(argc != 2){
        printf("too few arguments\n");
        exit(-1);
    }

    fp=fopen(argv[1], "r");
    if(fp == NULL){
        printf("failed to open file\n");
        exit(-1);
    }
    while(fgets(reader, MAXCHAR, fp) != NULL){ // !feof is not recommended search why

        n = strchr(reader, '\n');
        if(n != NULL){
            *n = '\0';
        }
        strcpy(tempList[index], reader);
        index++;
    }

    /*
    printf("%lu\n",sizeof(reader));     //256
    printf("%lu\n",sizeof(*reader));    //1
    printf("%lu\n",sizeof(*tempList));  //256
    printf("%lu\n",sizeof(**tempList)); //1
    */

    for(i=0; i<index; i++){
        printf("%s\n", tempList[i]);

    }
    qsort(tempList, index, sizeof(*tempList), myCompare);
    printf("\n\nAfter sorting\n\n");
    for(i=0; i<index; i++){
        printf("%s\n", tempList[i]);
    }
}

edited Jul 31, 2015 at 18:09

answered Jul 31, 2015 at 17:30

Soner from The Ottoman Empire

21.3k8 gold badges97 silver badges123 bronze badges

10 Comments

Weather Vane Over a year ago

Would have been more educative to point out the errors, but as it gives the correct output (passing strcmp to qsort before your edit), +1. Why did you change it?

Soner from The Ottoman Empire Over a year ago

Thanks for your advice, I've added as comment @WeatherVane

Weather Vane Over a year ago

MSVC gave no warning about the arguments of the function passed to qsort.

Soner from The Ottoman Empire Over a year ago

It's possible. I think the warning already can be ignored. But, XCode gives

Peter Cordes Over a year ago

WhozCraig's answer is better. It explains the actual problem (two-dimensional array of char, vs. array of char *). This answer has the right change to the args to qsort, but doesn't mention that change anywhere.

|

Collectives™ on Stack Overflow

Qsort array of strings in alphabetical order

3 Answers 3

2 Comments

2 Comments

10 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

2 Comments

2 Comments

10 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related