Why it is not validating string in c

Question

Below is the code to validate a string which should only contains alphabets, it is showing invalid for a valid string ..

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

int checkData(char[],int);


void main()
{

    char name[50];
    int i=0,flag,len;
    printf("Enter the name\n");
    fgets(name,50,stdin);
    len=strlen(name);

    flag=checkData(name,len);
     if(flag==1)
    {
        printf("Valid name");
    }
    else
    {
        printf("Invalid name");
    }

}

int checkData(char name[],int len )
{

 int i=0,flag=1;

    while(name[i]!='\0')
    {
        if(name[i]>='a' && name[i]<='z')
        {

        }
        else
        {
            flag=0;
            break;
        }

        i++;

    }
        return flag;
}

Use a debugger to step thru your code line by line. That is the right way to go about finding the problem for yourself. — kaylum, Jun 17 '17 at 06:52
No need to pass `len` and calculating it when work can be done with `name[i]!='\0'`. — GAURANG VYAS, Jun 17 '17 at 06:54
Suggest you read the man page for `fgets`. You will find it stores something interesting at the end of the string that you are not accounting for. Again, this is something you can easily spot for yourself if you do some basic debugging. — kaylum, Jun 17 '17 at 06:55

score 1 · Accepted Answer · answered Jun 17 '17 at 07:23

as kaylum told you in the comment, your answer is the documentation of the fgets function:

reads characters from stream and stores them as a C string into str until (num-1) characters have been read or either a newline or the end-of-file is reached, whichever happens first. A newline character makes fgets stop reading, but it is considered a valid character by the function and included in the string copied to str. A terminating null character is automatically appended after the characters copied to str. Notice that fgets is quite different from gets: not only fgets accepts a stream argument, but also allows to specify the maximum size of str and includes in the string any ending newline character.

Moreover there are some problems in your code: I mean, it works but you could just make it better:

GAURANG VYAS make you notice that the len parameter in your function is useless since you check in your while loop for the null character. With the length you could have easily used a for loop.
if(name[i]>='a' && name[i]<='z'){} : if you don't have to do anything with this check, use the not operator (!) and check only for the else statement.
again: the break keyword is strongly discourage (unless you use it in a switch statement) and it can be always replaced with an additional condition in the while loop. In your case you can just check in the while condition if flag has been set to 0.
Last one: instead of create a flag variable in main function you could have check for the return value of your function checkData in the if statement if(flag==1) directly in this way: if(checkData).

I repeat: these are not errors, it's a way to make your code more clear for you and for others.

Wait ... *"`break` is strongly discouraged [except in a `switch`]"*? WTF? First time I heard that. — pmg, Jun 17 '17 at 07:43
ok, you're right... maybe I exaggerated, but the point is that, in my opinion, `break` keyword make the code less readable especially when there are more nested loops. — Marco Luzzara, Jun 17 '17 at 07:49
`break` in my opinion is **way more readable** than any flag construct you could possibly build. It clearly says what happens. — tofro, Jun 17 '17 at 09:55
Well, I'm thinking about nested loops and in general you can control the flow of your code much better using flag variables. Actually you all are right if we are considering an easy code just like this one, I should have specified my point. — Marco Luzzara, Jun 17 '17 at 10:53

Rajeev Ranjan · Answer 2 · 2017-06-17T20:22:47.267

You are facing an issue because of fgets(). You need to truncate the trailing \n if you read the input using fgets. This trailing new line character causes the checks to fail. You would also need to pass a decremented length. len-1.

Below is the running code.

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

int checkData(char[],int);


void main()
{

    char name[50];
    int i=0,flag,len;
    printf("Enter the name\n");
    fgets(name,50,stdin);
    len=strlen(name);
    name[len-1] = '\0';

    flag=checkData(name,len-1);
     if(flag==1)
    {
        printf("Valid name");
    }
    else
    {
        printf("Invalid name");
    }

}

int checkData(char name[],int len )
{

 int i=0,flag=1;

    while(i < len)
    {
        if(name[i]>='a' && name[i]<='z')
        {
            i++;
            continue;
        }

        flag=0;
        break;

    }
        return flag;
}

Oh sorry! Cleaned up the increment of i while some cleanup of debug statements. Have it fixed now. — Rajeev Ranjan, Jun 17 '17 at 20:24

Rohan Kumar · Answer 3 · 2017-06-17T07:23:31.233

If you read a bit about fgets, you would come to know that it reads the complete line including (\n). In you function your flag resets to zero when it encounters the newline at the end since it fails your validation condition. I would suggest trim the newline at the end like this :

char *newLinePos;
newLinePos = strchr(name, '\n'); // Check if newline exists
if(newLinePos) *newLinePos = '\0'; // place null character at that position

Apart from that your main function must return an integer. It is considered a bad coding practice. Read this for more information. You can also use isalpha(name[i]) of the ctype library in your validation expression.

what if there was no newline (`'\n'`) character at the end of the string ? Does your solution work ? You'll end up trimming the last valid character. — , Jun 17 '17 at 07:18

Why it is not validating string in c

3 Answers3