why does strcmp() not work in my program?

Question

I am writing a program that takes multiple words as input and determines which word would come first and last if the words were listed in dictionary order. However my program does not fully work just yet.

Code:

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

void getword(void);

char str[20];
char smallest[20];
char largest[20];

int main(int argc, char *argv[]) {
    while (strlen(str) != 4) {
        getword();

        if (strcmp(str, smallest) < 0) {
            strcpy(smallest, str);
        } else
        if (strcmp(str, largest) > 0) {
            strcpy(largest, str);
        }
    }

    printf("smallest:%s\nlargest:%s\n", smallest,largest);

    return 0;
}

void getword(void) {
     printf("Enter a word: ");
     scanf("%s", str);
}

The user must enter words, I will assume the words are no longer than 20 characters long and if the user enters a 4 letter word then the program will stop.

The problem I have is that the first if statement (tests for smallest word in dictionary order) does not work. When the program is ran the output looks like this:

Enter a word: dog
Enter a word: zebra
Enter a word: rabbit
Enter a word: catfish
Enter a word: walrus
Enter a word: cat
Enter a word: fish
smallest:
largest:zebra

As you can see the 'smallest' word is not picked up and a simple printf statement in the if statement shows me that my program doesn't even enter the if statement at all, why is this?

What do you think are the *initial* values for smallest and largest? — wildplasser, Jun 19 '21 at 13:15
Your `smallest[20]` is an empty character array. Then you compare the empty against something, so the empty win. This is why you don't see any output at the smallest. — zerocukor287, Jun 19 '21 at 13:18
"will assume the words are no longer than 20 characters" --> off by 1. "no longer than 19". Rather than assume, use `fgets()`instead of `scanf()` and lop off the read `'\n'`. — chux - Reinstate Monica, Jun 19 '21 at 13:18
The first value you enter is both the smallest _and_ the largest to date. And, just FYI, `strcmp()` is working fine — it is your expectations that are mismatched with the logic you've written. — Jonathan Leffler, Jun 19 '21 at 13:19
I think you want `char smallest[20] = "~~~~~~~~~~~~~~~~~~~";` — Weather Vane, Jun 19 '21 at 13:19
Or: initialise before the loop, using memset() with 0xff and 0. — wildplasser, Jun 19 '21 at 13:23
`while (strlen(str) != 4){` what do you think `str` contains at the start of main? — phuclv, Jun 19 '21 at 14:26
Does this answer your question? [Problems with STRCPY and STRCMP when finding the smallest and largest problems](https://stackoverflow.com/questions/47842430/problems-with-strcpy-and-strcmp-when-finding-the-smallest-and-largest-problems) — Miraz, Jun 19 '21 at 19:06

score 2 · Answer 1 · edited Jun 20 '21 at 04:20

There are multiple problems:

you test the length of str before reading the word from the user, hence the 4 letter word will be handled.
you should protect the scanf("%s", str) as scanf("%19s", str) to avoid buffer overflow if a word longer than 19 characters is entered.
smallest is a global variable, thus is initialized as an empty string, so it is always smaller than any non empty input. You should avoid global variables.
if a single word in input, it will be both the smallest and the largest word, hence you should not have an else statement or make a special case of the first word.

Here is a modified version:

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

// read a word from the user, return the word length
int getword(char *str) {
    printf("Enter a word: ");
    *str = '\0';
    scanf("%19s", str);
    return strlen(str);
}

int main(int argc, char *argv[]) {
    char str[20];
    char smallest[20] = "";
    char largest[20] = "";

    while (getword(str) != 4) {
        if (*smallest == '\0' || strcmp(str, smallest) < 0) {
            strcpy(smallest, str);
        }
        if (strcmp(str, largest) > 0) {
            strcpy(largest, str);
        }
    }
    printf("smallest:%s\n", smallest);
    printf("largest:%s\n", largest);

    return 0;
}

Miraz · Answer 2 · 2021-06-19T19:10:09.530

You are comparing str with smallest or largest. But the problem is smallest and largest are initialized to empty string(being declared as global). That means str may contain "Dog"/"Zebra"/"cat"/"anything" while smallest contains ""(empty string). In this case, (strcmp(str,smallest) < 0) will always be false. As a result smallest will always remain same and in the end, result will be smallest = "". Your program is doing right that and printing empty string as result of smallest.

What you can do is:

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

void getword(void);

char str[20];      //str = ""
char smallest[20]; //smallest= ""
char largest[20];  //largest = ""


int main(int argc, char *argv[])
{
    //>--------extra part begins-->---------------
    scanf("%s", smallest);
    strcpy(largest,smallest);
    //<--------extra part ends--<---------------

    while (strlen(str) != 4)
    {
      getword();

      if (strcmp(str,smallest) < 0)
      {
          strcpy(smallest,str);
      }

      else if (strcmp(str, largest) > 0)
      {
          strcpy(largest,str);
      }
 }

    printf("smallest:%s\nlargest:%s\n", smallest,largest);

    return 0;
}

void getword(void){
     printf("Enter a word: ");
     scanf("%s", str);
}

"But the problem is smallest and largest are not initialized. " --> with OP's `char smallest[20]; char largest[20];` outside a function, both are fully initialized to all zeros. — chux - Reinstate Monica, Jun 19 '21 at 17:14
@chux-ReinstateMonica does any variable declared outside a function is initialized to zero? I didn't know about it? — Miraz, Jun 19 '21 at 19:03
Global variables as well as static local variables without an initializer are initialized to the `0` value of their type before the `main` function is run. integers are initialized to `0`, floating point objects to `0.0`, pointers to the null pointer. Aggregates have all member initialized this way. For `unions`, the first member is initialized. Note that this may be different from initializing objects to all bits zero as performed by `calloc` and `memset`, but for most current architectures, `0.0` and the null pointers have all their bits cleared. — chqrlie, Jun 19 '21 at 20:39

why does strcmp() not work in my program?

2 Answers2