HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Get initials from name input by user

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
userinitialsinputgetnamefrom

Problem

This was an exercise from cs50. The goal was to get the initials in upper letter of a name that we got from user. So for "barack obama" we would get "BO", input like "abc" should output "A" and so on. I am interested in knowing what I can do to simplify it or be more efficient.

Do I need the check against '\0' in the while loop? I ask because the way I see it, when a user inputs a name, there will always be a '\n'(right ?), so it gets out of the while loop when its position has '\n' and then out of for loop so '\0' is never actually checked. Let me know if logic is right. I kept it there just in case.

#include 
#include 

#define NAME_LEN 80 + 1

int main(void)
{
    char userName[NAME_LEN], userInitials[NAME_LEN];
    int i, pos = -1;

    // ask user for a name
    fgets(userName, NAME_LEN, stdin);
    for(i = 0; userName[i] != '\n' && userName[i] != '\0'; i++)
    {
        if (userName[i] >= 'a' && userName[i] <= 'z')
        {
            userInitials[++pos] = userName[i] - ('a' - 'A');
        }
        else
        {
            userInitials[++pos] = userName[i];
        }
        while ((!isblank(userName[i])) && (userName[i] != '\n') && (userName[i] != '\0'))
        {
            i++;
        }
    }
    userInitials[++pos] = '\0';
    printf("%s\n", userInitials);

}

Solution

The compiler doubles as a code-analysis tool

Compiling your file like: gcc usename.c -Wall -pedantic Warns you of a bad practice in your code:

usename.c:12:5: warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result [-Wunused-result]
     fgets(userName, NAME_LEN, stdin);


You should use the return value to check if input reading happened correctly.

Small helper function to increase readibility

The line

userInitials[++pos] = userName[i] - ('a' - 'A');


is not so immediate to understand, I suggest writing a tiny function:

char to_uppercase(char letter) {
    return letter - ('a' - 'A');
}


and calling it like:

userInitials[++pos] = to_uppercase(userName[i]);


The C99 Standard and variable scope

All major C compiler support C99, I suggest using the appropriate flag to enable it and write:

for(int i = 0; userName[i] != '\n' && userName[i] != '\0'; i++)


to shorten the scope of i.

Excellent parametrization

You defined a constant for NAME_LEN and named it meanigfully instead of sprinkling a random number all over your code, excellent!

For further improvement I suggest using a

static const int NAME_LEN = 80 + 1


For reasons discussed here.

Code Snippets

usename.c:12:5: warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result [-Wunused-result]
     fgets(userName, NAME_LEN, stdin);
userInitials[++pos] = userName[i] - ('a' - 'A');
char to_uppercase(char letter) {
    return letter - ('a' - 'A');
}
userInitials[++pos] = to_uppercase(userName[i]);
for(int i = 0; userName[i] != '\n' && userName[i] != '\0'; i++)

Context

StackExchange Code Review Q#96567, answer score: 5

Revisions (0)

No revisions yet.