C input and return

Question

I am trying to work on a employee management system that takes employee information as input and prints the output accordingly.

Structure:

struct employee
{
    char empId[10];
    char name[20];
    char empType[10];
    int dd, mm, yyyy;
};

Program to check if the string contains only integers:

void checkValidId(char *id)
{
    int count = 0, i;

    for(i = 0; i < strlen(id); i++)
    {
        if(id[i] >= '0' && id[i] <= '9')
        {
            count++;
        }
    }
    if(count == strlen(id))
        // continue executing next statements of input() function
    else
        // take the empId input again
}

Function to take input:

struct employee input()
{
    struct employee e;

    printf("Enter Employee ID: ");
    scanf("%s", &e.empId); 
    checkValidId(e.empId);

    // next statements ....
}

Here I am trying to check if the string input that is empId contains only integers which is done by checkValidId(). If yes then the program continues executing next statements. Else I want to take the input empId again. How to do it.

Requesting help!

Thanks!

Answer 1

Include the header <ctype.h>, and use isdigit to test if a character is in the set '0' ... '9'.

Include the header <stdbool.h>, and change the signature of checkValidId to

bool checkValidId(const char *id)

in order to indicate a result to the calling function.

In checkValidId, loop through each character of the string. If the current character is not a digit, immediately return false from the function. If the loop finishes, all the characters must be digits, so you can then return true.

Note that there is no reason to call strlen here. Simply loop until the current character is the null-terminating byte.

---

&e.empId is of type char (*)[10], that is a pointer-to-array-of-10-char. The scanf specifier "%s" expects the type char *, or pointer-to-char. An array will decay to a pointer to its first element when passed to a function, so the the 'correct' way to call scanf here is scanf("%s", e.empId);.

That said, you must check that the return value of scanf is the expected number of conversions, otherwise you will be operating on indeterminate data.

Additionally, an unbound "%s" is as dangerous as the gets function, as it does not know when to stop reading data, and will easily overflow the provided buffer. You must provide a maximum field width to prevent scanf from reading too many characters. This should be at most the size of your buffer minus 1, leaving room for the null-terminating byte.

An example of using scanf safely:

if (1 != scanf("%9s", e.empId)) {
    /* handle input stream error */
}

Note that when scanf fails to perform a conversion, the problem data is left in the stream. Recovering from bad input with scanf is very hard, and for that reason a line-based approach to user input is generally advised. This can be accomplished with fgets.

If there is room in the buffer, fgets includes the newline character when read. See Removing trailing newline character from fgets() input for an example usage of strcspn, which can also be used as a way to get the length of the input.

---

To repeatedly ask the user for input, use an infinite loop. Only break from the loop when the user correctly enters the requested data, or a critical error occurs.

Here is an example program, using the methods discussed:

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

struct employee {
    char empId[10];
    /* ... */
};

bool checkValidId(const char *id)
{
    for (size_t i = 0; id[i] != '\0'; i++)
        if (!isdigit((unsigned char) id[i]))
            return false;

    return true;
}

bool getEmployee(struct employee *e, FILE *stream)
{
    char buffer[256];

    while (1) {
        printf("Enter employee ID: ");

        if (!fgets(buffer, sizeof buffer, stream))
            return false;

        size_t length = strcspn(buffer, "\r\n");
        /* remove the newline */
        buffer[length] = '\0';

        if (!length)
            fprintf(stderr, "Invalid ID. Zero length.\n");
        else if (length >= sizeof e->empId)
            fprintf(stderr, "Invalid ID. Too long.\n");
        else if (!checkValidId(buffer))
            fprintf(stderr, "Invalid ID. Contains non-digit character(s).\n");
        else {
            strcpy(e->empId, buffer);
            break;
        }

        puts("Try again.");
    }

    return true;
}

int main(void)
{
    struct employee emp;

    if (!getEmployee(&emp, stdin))
        return 1;

    printf("ID: <%s>\n", emp.empId);
}

Interacting with this program:

Enter employee ID: foobar
Invalid ID. Contains non-digit character(s).
Try again.
Enter employee ID: 
Invalid ID. Zero length.
Try again.
Enter employee ID: 1234567890
Invalid ID. Too long.
Try again.
Enter employee ID: 42
ID: <42>

Answer 2

1. checkValidId should return int so it can tell the caller whether the id is valid [or not].
2. A caller (e.g. input) should loop if the return value of checkValidId indicates a bad id.
3. No need to use strlen at all.
4. No need to compare lengths. A simpler algorithm can be used.
5. We can just stop the loop if an invalid char is detected

Here is the refactored code:

struct employee {
    char empId[10];
    char name[20];
    char empType[10];
    int dd, mm, yyyy;
};

// Program to check if the string contains only integers:
// RETURNS: 1=input valid, 0=retry
int
checkValidId(char *id)
{
    int valid = 0;

    for (int i = 0; id[i] != 0; i++) {
        valid = (id[i] >= '0') && (id[i] <= '9');
        if (! valid)
            break;
    }

    return valid;
}

// Function to take input:
struct employee
input(void)
{
    struct employee e;

    while (1) {
        printf("Enter Employee ID: ");
        scanf("%s", &e.empId);
        if (checkValidId(e.empId))
            break;
    }

    // next statements ....
}