Sorting an array of strings in an alphabetical order using pointers

Question

I have a project where I have to create a program that would let users input names in any order. Then the program displays the names alphabetical order. Also all of this has to be done using pointers. Now my attempt at the program prompts the user to put in the names and it displays them, but I can't get it to sort them for some reason. Can someone please help me?

Here's my attempt at the program:

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

int main() {
  int list;
  char *names[20];
  char str[20];
  printf("Enter the number of names: ");
  scanf("%d", &list);
  fflush(stdin);
  for (int i = 0; i < list; i++) {
    printf("Enter name %d: ", i + 1);
    // gets(str);
    scanf("%[^\t\n]s", str);
    fflush(stdin);
    names[i] = (char *)malloc(strlen(str) + 1);
    strcpy(names[i], str);
  }
  void sortNames();
  for (int i = 0; i < 5; i++)
    printf("%s\n", names[i]);
  return 0;
}

void sortNames(char **name, int *n) {
  int i, j;
  for (j = 0; j < *n - 1; j++) {
    for (i = 0; i < *n - 1; i++) {
      if (compareStr(name[i], name[i + 1]) > 0) {
        char *t = name[i];
        name[i] = name[i + 1];
        name[i + 1] = t;
      }
    }
  }
}

int compareStr(char *str1, char *str2) {
  while (*str1 == *str2) {
    if (*str1 == '\0' || *str2 == '\0')
      break;

    str1++;
    str2++;
  }
  if (*str1 == '\0' && *str2 == '\0')
    return 0;
  else
    return -1;
}

I assume that you're writing your own `compareStr` function for didactic reasons, but on the off chance you're not you may wish to use `strcmp` (from `string.h`) instead. — kopecs, Mar 14 '20 at 02:39
Can't you just use `qsort`? Or are you required to write your own sort function? — Tom Karzes, Mar 14 '20 at 02:42
@kopecs Yes, I am writing my own compareStr, it's a requirement. — A aqua, Mar 14 '20 at 02:46
I'm not clear on why you're taking `n` as a pointer in `sortNames`. Its also notable that your `compareStr` function never returns 1. What's the intended handling of `str1` being greater? — kopecs, Mar 14 '20 at 02:48
@H.S.I tried removing the fflush(stdin) statements, but that appears to make the program run worse. Am I missing something? — A aqua, Mar 14 '20 at 02:50
You're not actually calling `sortNames`. You're just declaring it, using old-style syntax that lacks a prototype for the arguments (which you should never do). So as soon as you exit the loop that inputs the names, you're immediately printing them out. `sortNames` is never called. — Tom Karzes, Mar 14 '20 at 02:50

score 1 · Accepted Answer · answered Mar 14 '20 at 03:09

Focusing only on the issues with sorting, the main one is that you never call the sorting function you later define. The line

void sortNames();

serves only to declare a function with the identifier sortNames which takes any number of arguments of any types (probably not quite what you want to do). I would recommend revising this line to instead be

sortNames(names, list); // Not &list because I'm about to suggest not taking it as a pointer

Then for the sortNames function itself, I'm not totally clear on why you take the length of the array to be sorted as a pointer instead of just passing the int itself. I'd recommend revising this function to instead be

void sortNames(char **name, int n) {
  int i, j;
  for (j = 0; j < n - 1; j++) {
    for (i = 0; i < n - 1; i++) {
      if (compareStr(name[i], name[i + 1]) > 0) {
        char *t = name[i];
        name[i] = name[i + 1];
        name[i + 1] = t;
      }
    }
  }
}

One issue with this as it stands though is that the expression compareStr(name[i], name[i + 1]) > 0 is always false. This is due to the fact that compareStr only ever returns 0 or -1. You could fix this by re-writing compareStr to correctly handle the case where the *str1 > *str2. One possible way to do this could be

int compareStr(char *str1, char *str2) {
    if (*str1 == '\0' && *str2 == '\0') {
        return 0;
    } else if (*str1 > *str2) {
        return 1;
    } else if (*str1 < *str2) {
        return -1;
    }
    return compareStr(str1 + 1, str2 + 1);
}

Although if you're writing this to learn I'd suggest trying to revise your current iterative solution instead of just copying & pasting this version.

Finally, because you want to use these functions before you've defined them you should either move their definitions prior to when they're used (i.e., have compareStr, then sortNames and then main) or provide a forward declaration at the start of your file for these functions, i.e., add

void sortNames(char **name, int n);
int compareStr(char *str1, char *str2);

above your main.

As others have noted you probably want to avoid fflush(stdin) as its undefined behaviour and I would recommend against casting the result of malloc.

score 1 · Answer 2 · answered Mar 14 '20 at 03:30

The problem here is that the function compareStr will never return a value greater than 0. It just tells you if the 2 strings are similar or not.

For sorting, you need to add an extra logic as follows:

int compareStr(char *str1, char *str2) {
    while (*str1 == *str2) {

        if (*str1 == '\0' || *str2 == '\0')
            break;

        str1++;
        str2++;
    }
    if (*str1 == '\0' && *str2 == '\0'){
        return 0;
    }
    else if(*str1 > *str2){
        return 1;
    }else{
        return -1;
    }
}

Besides this, you have to make a call to function sortNames as sortNames(names, &list) and make sure that function definitions are written in proper order or make use of function declarations.

Sorting an array of strings in an alphabetical order using pointers

2 Answers2