Capitalised initials of username

Given a list of words, this program outputs the starting letters of the words capitalized.

I was reviewing this question that solves the same task, but over time so many changes accumulated, that I think my programme bears little resemblance to the original one, so instead of posting it as an answer, I ask you for further improvement.

#include 
#include 
#include 
#include 
#include 

const int NAME_LEN = 160 + 1;

bool is_letter(char ch) 
{
    return ch >= 'a' && ch <= 'z';
}

bool is_terminating_char(char ch) 
{
    return ch == '\n' || ch == '\0';
}

const char * first_letters_only_capitalized(char words[NAME_LEN]) {

    char *capitals = malloc(sizeof(char) * NAME_LEN);
    int pos = -1;

    for(int i = 0; ! is_terminating_char(words[i]); i++) 
    {
        capitals[++pos] = is_letter(words[i]) ? toupper(words[i]) : words[i];

        while (is_letter(words[i])) 
        {
            i++;
        }
    }

    capitals[++pos] = '\0';

    return capitals;
}

int main() 
{
    char user_name[NAME_LEN], user_initials[NAME_LEN];

    puts("Enter your name, I will output the initials capitalized: ");
    if ( ! fgets(user_name, NAME_LEN, stdin))
    {
        puts("Error reading input.");
        return 1;
    }

    strcpy(user_initials, first_letters_only_capitalized(user_name));
    printf("%s\n", user_initials);
}

Example session:

Enter your name, I will output the initials capitalized:
foo bar buz
FBB


Here are some things that may help you improve your code:

Avoid memory leaks

The first_letters_only_capitalized function allocates memory (or tries to, see next suggestion) and returns a pointer to that memory, but it is never freed by the calling function. Instead of using strcpy into a fixed size string, you could instead use the return value directly:

char *user_initials = first_letters_only_capitalized(user_name);
if (user_initials != NULL) {
    puts(user_initials);
}
free(user_initials);

Check for failed memory allocation

Whenever a program allocates memory explicitly, as with using malloc or calloc, it should check to make sure that the allocation succeeded before using the resulting pointer. When malloc fails, it returns NULL, so you could modify your program like this:

char *capitals = malloc(sizeof(char) * NAME_LEN);
if (capitals == NULL) {
    return capitals;
}

Use const appropriately

One of the functions in your program is declared like this:

const char * first_letters_only_capitalized(char words[NAME_LEN])

But I think it should be this instead:

char * first_letters_only_capitalized(const char words[NAME_LEN])

The reason is that the first signature says that that words might be altered but that the returned pointer may not be. That seems backwards from the real situation which is that words is not altered by the program (and therefore should be declared const) but that the return value may be used by the caller in any way, including by using free.

Use consistent formatting

Using consistent formatting helps readers of your code understand it without distraction. This code is mostly well formatted, but the opening brace of the first_letters_only_capitalized is on the same line, which is unlike all the other functions in the program.

Reconsider the sense of boolean functions

The only time is_termininating_char is used, its return value is negated. That suggests the function sense should be inverted:

bool is_not_terminating_char(char ch) 
{
    return ch != '\n' && ch != '\0';
}

Allow mixed case input

As the program stands, if the user enters "John Smith" the program returns "JOSM" which is rather odd. Instead, alter the program to allow for mixed case input. It may also be interesting to consider other cases such as hyphenated last names and last names such as "von Trapp" in which part of the name is never capitalized.

Trim leading space

The program currently does not trim non-alphabetic leading characters including spaces, so an input of " #!%! Al Alpha" yields an output of " #!%! ALAL".

Use standard functions where appropriate

Instead of writing your own, it would make more sense to me to use standard function isalpha rather than writing your own is_letter function.

Consider using pointers

Using pointers in your first_letters_only_capitalized function would make it more idiomatic C in my opinion. Your first_letters_only_capitalized function might look like this.

char * first_letters_only_capitalized(const char words[NAME_LEN]) 
{
    char *capitals = malloc(sizeof(char) * NAME_LEN);
    if (capitals == NULL) {
        return capitals;
    }
    char *init = capitals;

    for(const char *ch = words; is_not_terminating_char(*ch); ++ch)
    {
        if (isalpha(*ch)) {
            *init++ = toupper(*ch);
        }
        while (isalpha(*ch)) {
            ++ch;
        }
    }
    *init++ = '\0';

    return capitals;
}

Avoid overly long names

In writing this review, I typed first_letters_only_capitalized nine times. It's exhausting! Show some mercy to other programmers and use shorter names, although using a very long name for a function whose purpose is to shorten names might be amusing irony to some.

StackExchange Code Review Q#96576, answer score: 8