1
\$\begingroup\$

I took an entire line of input in the form of a string, and tokenized the string into substrings and obtained the first string as the first name and the second as the last name. This program as it stands is very basic and limited however my primary focus was on the creation of generic functions to do all the heavy lifting (generic as in they are not specific to the program and could be used elsewhere I've had several problems in the past where my functions are to narrowed on a specific problem, so please tell me how I did!).

The function for parsing the string, get_substr(), most likely can, and should, be improved and I am hoping for better ideas on how to do it!

/*Input name: first, last. Output name: Last, first*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

#define MAXSIZE 1024

bool readline(char *line, const size_t buffer_len);
bool hasDigits(const char *str);
bool hasPunct(const char *str);
void chop_newline(char *str);
char *remove_leading_whitespace(char *str);
char *get_substr(char *str, char **endptr, const char *str_dim);


bool readline(char *line, const size_t buffer_len) {
    if (fgets(line, buffer_len, stdin) != NULL) {
        chop_newline(line);
        if (line[0] != NULL) {
            return true;
        }
    }
    return false;
}

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

bool hasPunct(const char *str) {
    for (size_t i = 0; str[i] != '\0'; ++i) {
        if (ispunct((unsigned char)str[i])) {
            return true;
        }
    }
    return false;
}

void chop_newline(char *str) {
    str[strcspn(str, "\n")] = '\0';
}

char *remove_leading_whitespace(char *str) {
    if (str) {
        while (isspace(*str) && str) str++; //remove whitespace
    }
    return str;
}

/*
get_substr

@param **endptr, set to next character after a diliminator in string str
@param *str_dim, list of diliminators used to split string ex: " !,.-?"

when called, absorb incoming whitespace ,read until first diliminator, return substring,
and set **endptr equal to the rest of the string. If endptr is not NULL, then string has more to parse,
returns NULL if nothing was parsed else returns c-string
*/

char *get_substr(char *str, char **endptr, const char *str_dim) {
    char *substr = NULL;
    remove_leading_whitespace(str);
    substr = strtok(str, str_dim);
    *endptr = strtok(NULL, "\0");

    return substr;
}

int main() {
    char str_name[MAXSIZE];
    char *last_name = NULL, *first_name = NULL, *endptr = NULL;

    printf("Please enter your first name followed by your last name: ");
    for (;;) {
        if (readline(str_name, sizeof str_name)) {
            if (!hasDigits(str_name) && !hasPunct(str_name)) {
                break;
            }
            printf("\nTry again, name must contain letters and cannot contain numbers or punctuation...\n");
        }
        else {
            printf("\nEmpty string please try again...\n");
        }
    }

    first_name = get_substr(str_name, &endptr, " ");
    last_name = get_substr(endptr, &endptr, " "); 

    if (endptr) {
        printf("Error, too much input, expected: First name Last name\n %s was not saved...\n", endptr);
    }
    else if (!last_name) {
        printf("Error must have a first and last name...\n");
    }
    else {
        printf("Your name is: %s, %s\n", last_name, first_name);
    }

    printf("Press any key to continue...\n");
    getch();
}
\$\endgroup\$
2
  • \$\begingroup\$ if (line[0]) or if (line[0] != '\0') would have my preference. And do you really want an empty line to yield false? An other line could follow. \$\endgroup\$
    – Joop Eggen
    Oct 12 '16 at 22:59
  • \$\begingroup\$ Falsehoods programmers believe about names. \$\endgroup\$
    – Bob
    Oct 13 '16 at 4:11
2
\$\begingroup\$
  1. Note that real last names contain -, ', spaces and likely other characters.

  2. The below tests in the negative. Code accepts control characters like '\t' as legitimate name characters. Better to test in the positive.

    // if (!hasDigits(str_name) && !hasPunct(str_name)) {
    //   break;
    //  }
    
    // Untested alternative, maybe also incorporate isalpha()
    size_t bad = strspn(str_nam, "abc(fill in the rest)XYZ'-");
    if (str_nam[bad]) Handle_Bad();
    
  3. if (line[0] != NULL) is not guaranteed to work as desired. NULL need not have the value of 0. Only that (void*)NULL compares equally to 0. Besides NULL is best used in a pointer context and line[0] is a char, not pointer.

    Use if (line[0]) or if (line[0] != '\0') as commented by @Joop Eggen

  4. Good used of bool and size_t in hasDigits() and others. Candidate simplification

    bool hasDigits(const char *str) {
        while (*str) {
            if (isdigit((unsigned char) *str++)) {
                return true;
            }
        }
        return false;
    }
    
  5. Inconsistent naming style:

    // lower case r-lower case l
    bool readline(char *line, const size_t buffer_len) {
    // lower case h-upper case D
    bool hasDigits(const char *str) {
    // lower case c-underscore-lower case n
    void chop_newline(char *str) {
    
  6. Missing #include <ctype.h> for is...() functions.

  7. getch() is not a standard C library function.

\$\endgroup\$
2
\$\begingroup\$

The function is maybe too ambitious in doing two things.

char *get_substr(char *str, char **endptr, const char *str_dim) {
    str = remove_leading_whitespace(str);
    char *substr = strtok(str, str_dim);
    *endptr = strtok(NULL, "\0");
    return substr;
}

I would keep the API simple and leave out endptr.

\$\endgroup\$
1
  • \$\begingroup\$ Good functional correction with str = remove_leading_whitespace(str); \$\endgroup\$ Oct 12 '16 at 23:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.