My if statement returns unexpected results

Question

I am learning C. What I want my program to do is prompt for three variables compare them to specific values using if statement and return true and print a welcome message only if (all) comparisons are true. Any thing else return false and print a sorry message. But when I compile and run the code, which is shown below.

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

int main ()
{
    int years;
    char name[15];
    char gender[7];
    char evaluation[7];
    char age[2];
    printf ("enter name, please     ");
    fgets (name, 15, stdin);
    printf ("enter gender, please     ");
    fgets (gender, 7, stdin);
    printf ("enter evaluation, please     ");
    fgets (evaluation, 20, stdin);
    printf ("Please enter age      ");
    fgets (age, 2, stdin);

    years = strlen (age);

    {
        if ((strcmp (gender, "male") == 1)
            && (strcmp (evaluation, "good") == 1) && years < 50)

            printf ("welcome volunteer, %s . You're fit to participate\n",
                    name);

        else
            printf ("Sorry, %s. You're not fit to participate/n", name);
    }

    return (0);
}

-It compiles with no errors and no warnings. But, - the if statement returns true if any of the first two comparisons, gender and evaluation, is true.

but, - it returns false when all are false. and - when the first two comparisons ,gender and evaluation,are both false and the third comparison ,age, is true it returns false.

I would like to know what's wrong with my if statement. Thanks for your help

Answer 1

You're not hitting your conditions here because you're expecting a different return from strcmp when you get a match.

From the strcmp manpage:

The strcmp() and strncmp() functions return an integer less than, equal to, or greater than zero if s1 (or the first n bytes thereof) is found, respectively, to be less than, to match, or be greater than s2.

Try testing for negation on the strings used in strcmp.

if (!strcmp(gender_str, "male"))

Answer 2

By trying to limit the buffer size to just what will fit, you are unwittingly shooting yourself in the foot. For example char gender[7]; will hold female\0, but what happened with the '\n' at the end of the input?

It remains unread in the input buffer (stdin here), just waiting to cause you problems on your next call to fgets (ever wonder why when you entered female your code seemed to skip the age prompt?) What happens if a cat steps on the keyboard and enters kkkkkkkkkkkkkkkkkkkkkkkkk for gender, or a nutty user enters you go figure it out!?

Lesson one, don't try and micro-manage the input array size when taking user input. Choose adequately sized buffers for your input (64, 128, or 256 chars are fine). If you are on an embedded system with very limited memory, sure, then cut back, but for normal use, a few extra bytes never hurts.

Lesson two -- always check that the full line has been read and that additional characters do not remain unread just waiting to screw you over on your next read. How do you do this? Simple, fgets reads up to and including the next '\n' (or the maximum number of characters specified in size, including the nul-byte). If you check the last character and it isn't a '\n', you have experienced a short-read and there are additional characters in the input buffer that remain unread.

Lesson three -- fgets reads up to and including the next '\n'. What do you think happens when you compare:

if (strcmp ("male\n", "male") == 0)

You will need to check for, and remove the '\n' from the end of the input if it is present.

How? Since this is something you will be doing for every input, it would make sense to have a single function you could call that would check if the newline was present at the end of the input and if so, overwrite the '\n' with a nul-terminating character (which is simply 0 or the equivalent '\0' form). If the '\n' isn't present in your input, you know it remains unread in the input buffer, so you need to read & discard all remaining characters in that line of input if the remainder is unneeded or read & process the input if it is. You can do that simply with something like:

void check_eol (char *s)
{
    if (!s || !*s) return;               /* validate pointer & non-empty s */
    if (*s == '\n') { *s = 0; return; }  /* avoid strlen call if 1st is \n */

    size_t len = strlen (s);     /* get lenth and check if last char is \n */
    if (s[len - 1] == '\n')
        s[len - 1] = 0;             /* if so, overwrite '\n' with nul-char */
    else {            /* otherwise '\n' remains in input buffer, read rest */
        int c;        /* of line, to prevent failure on next call to fgets */
        while ((c = getchar()) != '\n' && c != EOF) {}
    }
}

(you can add a FILE * parameter and use fgetc in place of getchar() if you are reading from other than stdin)

Putting all the pieces of the puzzle together, and leaving your age and gender arrays small to illustrate the point, along with a few other tips, you can do something like the following with your code:

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

/* use constants instead of 'magic' numbers in your code.
 * since you know the limit of age and gender, you can tailor
 * the size, otherwise, it is better to create a buffer large
 * enough to handle any name or evaluation.
 */
enum { MAXA = 4, MAXG = 8, MAXE = 32, MAXC = 256 };

void check_eol (char *s);

int main (void) {

    /* int years = 0; */
    char name[MAXC] = "",      /* initializing char arrays all 0 can insure */
        gender[MAXG] = "",     /* nul-terminated strings if you forget :)   */
        evaluation[MAXE] = "",
        age[MAXA] = "";

    printf ("enter name, please (max: %3d)       : ", MAXC - 1);
    fgets (name, MAXC, stdin);
    check_eol (name);

    printf ("enter gender, please (max: %3d)     : ", MAXG - 1);
    fgets (gender, MAXG, stdin);
    check_eol (gender);

    printf ("enter evaluation, please (max: %3d) : ", MAXE - 1);
    fgets (evaluation, MAXE, stdin);
    check_eol (evaluation);

    printf ("Please enter age (max: %3d)         : ", MAXA - 1);
    fgets (age, MAXA, stdin);
    check_eol (age);
    if (*age < '0' || '9' < *age) {     /* make sure 1st char is a number */
        fprintf (stderr, "error: non-numeric age entered.\n");
        return 1;
    }

    // years = strlen (age);   /* WTF?? */

    if ((strcmp (gender, "male") == 0)
        && (strcmp (evaluation, "good") == 0) && *age < 5 + '0')
        printf ("\nwelcome volunteer, %s . You're fit to participate\n",
                name);
    else
        printf ("\nSorry, '%s'. You're not fit to participate\n", name);

    return (0);
}

/** check for '\n' at end of 's', overwrite with 0 if present, otherwise
 *  read remaining chars from stdin.
 */
void check_eol (char *s)
{
    if (!s || !*s) return;               /* validate pointer & non-empty s */
    if (*s == '\n') { *s = 0; return; }  /* avoid strlen call if 1st is \n */

    size_t len = strlen (s);     /* get lenth and check if last char is \n */
    if (s[len - 1] == '\n')
        s[len - 1] = 0;             /* if so, overwrite '\n' with nul-char */
    else {            /* otherwise '\n' remains in input buffer, read rest */
        int c;        /* of line, to prevent failure on next call to fgets */
        while ((c = getchar()) != '\n' && c != EOF) {}
    }
}

(note: the check for age < 50 just checks that the first character of age is less than 5, the test can simply be written *age < '5', is the way it is currently written equivalent? why/why not?)

Example Use/Output

Fit - male, good, < 50

$ ./bin/rfmt
enter name, please (max: 255)       : Somebody With Avery-Longname
enter gender, please (max:   7)     : male
enter evaluation, please (max:  31) : good
Please enter age (max:   3)         : 49

welcome volunteer, Somebody With Avery-Longname . You're fit to participate

Not fit - male, good, = 50

$ ./bin/rfmt
enter name, please (max: 255)       : Somebody With Avery-Longname
enter gender, please (max:   7)     : male
enter evaluation, please (max:  31) : good
Please enter age (max:   3)         : 50

Sorry, 'Somebody With Avery-Longname'. You're not fit to participate

Not fit - unknown, good, < 50

$ ./bin/fgets_user_input
enter name, please (max: 255)       : Somebody With Avery-Longname
enter gender, please (max:   7)     : unknown
enter evaluation, please (max:  31) : good
Please enter age (max:   3)         : 49

Sorry, 'Somebody With Avery-Longname'. You're not fit to participate

Look things over and let me know if you have any questions. Hope some of this helps.

Answer 3

You have a few things wrong. As others have said strcmpshould return 0 if the strings are equal.

Second thing, remove the newline character after you read in the values

 gender[strlen(gender) - 1] = '\0';
 evaluation[strlen(evaluation) - 1] = '\0';

Third, years = strlen (age); should be years = atoi(age)

Answer 4

strcmp doesn't work the way you think it works. It returns 0 (not 1) if the strings are the same, and a value less than one (which is usually -1, but do not rely on it) if the first has a lower dictionary sorting than the second string, and a value larger than 0 (which may or may not be 1) if the second is higher.

So when you checked for equality of strings, what you actually checked for was for a very specific inequality of strings. They is probably the reason your program misbehaves.

In general, whenever a function returns unexpected results, make sure to go over that function's documentation to make sure you are not misusing it.