patterncMinor
Get initials from name input by user
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
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:
You should use the return value to check if input reading happened correctly.
Small helper function to increase readibility
The line
is not so immediate to understand, I suggest writing a tiny function:
and calling it like:
The C99 Standard and variable scope
All major C compiler support C99, I suggest using the appropriate flag to enable it and write:
to shorten the scope of
Excellent parametrization
You defined a constant for
For further improvement I suggest using a
For reasons discussed here.
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 + 1For 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.