3
\$\begingroup\$
#include <string.h>
#include <cs50.h>
#include <stdio.h>
#include <ctype.h>

int main(void)
{
    // store user input
    char * name = get_string();

    // print out the first character of firts string
        printf("%c", toupper(name[0]));

    // iterate through name
    for (int i = 0, n = strlen(name); i < n; i++)
    {
        // if i'th character is a space print next character as uppercase
        if (name[i - 1] == ' ')
        {
            printf("%c", toupper(name[i]));

        }

    }
    // print new line following initials
    printf("\n");

}

It does what it's supposed to do but I'm concerned about the style of the program.

\$\endgroup\$
  • 1
    \$\begingroup\$ I've changed your title because, The site standard is for the title to simply state the task accomplished by the code. Please see How to ask for examples \$\endgroup\$ – Ludisposed Nov 1 '17 at 9:12
2
\$\begingroup\$

Your code is buggy. It might seem to work, but accessing name[-1] invokes undefined behavior. You do that in the first iteration of the for loop. To fix it, start at i = 1 instead of i = 0.

The next instance of undefined behavior is when you call toupper(name[i]). The functions from <ctype.h> must not be called with a char as their argument. It must always be an unsigned char or the value of the macro EOF. To fix this, call toupper((unsigned char) name[i]).

The proper type for array indexes is not int, but size_t. Therefore the loop should start with for (size_t i = 1, n = strlen(name).

You have a typo: firts instead of first.

Other than that, the code looks good. In some months you will be able to remove all the comments because they are obvious to you. But until then, they should stay.

To make your program even more correct, you could change the condition name[i - 1] == ' ' to be more precise: !isalpha((unsigned char) name[i - 1]) && isalpha((unsigned char) name[i]). Then you won't print an "uppercase space" if the input contains several spaces in a row.

\$\endgroup\$
1
\$\begingroup\$

C strings are simply an array of bytes that is terminated by a NUL byte. How, then, does strlen() work? It contains a loop to walks along the array until it finds the NUL terminator.

But your code also tests every character of the array until it reaches the end. It would therefore be simpler and more efficient for you to stop when you encounter the '\0' terminator, instead of incrementing a counter.


To print a single character, it would be simpler to use the putchar(…) function rather than printf("%c", …) or printf("\n").

\$\endgroup\$

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.