1

I have to capitalize the first letter of every word (words are separated by a space) into a given array of char. I wrote the code but I can't figure out why it's not working nor displaying anything in output.

Here's the code:

void LetterCapitalize(char *str) {
    char *str2;
    int i = 0;
    str2[i] = toupper(str[0]);
    i++;
    while (str[i]) {
        if (str[i] == ' ') {
            str2[i] = str[i];
            str2[i + 1] = toupper(str[i] + 1);
            i += 2;
        } else {
            str2[i] = str[i];
            i++;
        }
    }
    printf ("%s", str2);
}

And here's the main:

int main(void) {
    char stringa[16] = "some string here";
    LetterCapitalize(stringa);
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
al366io
  • 87
  • 8
  • 4
    `char *str2;` is uninitialized, you need to allocate some memory to it or take an output buffer as a second parameter. – Sergey Kalinichenko May 26 '22 at 17:43
  • 1
    `str2` is also superfluous. You could just walk the string with `str` in that function. not like you have to revert back to the beginning of the string for anything. And your code doesn't account for multi-whitespace separation, e.g. `"one two three four"` is *not* going to do what you expect/want. – WhozCraig May 26 '22 at 17:45
  • What happens if there are two spaces between a pair of words? – Jonathan Leffler May 26 '22 at 17:46
  • 1
    `stringa` is one character short to be a proper C string. There is no space for the null terminator. Better let the compiler figure out the length: `char stringa[] = "whatever";` – M Oehm May 26 '22 at 17:46
  • @SergeyKalinichenko okay done, but now it still doesnt do what i want. Gives me some weird characters. – al366io May 26 '22 at 17:47
  • @WhozCraig and Jonathan Leffler, yeah i know, the exercise told me to consider only one space between the words. Code still doesnt work – al366io May 26 '22 at 17:49
  • `stringa[16]` does not have enough memory for the null terminator. You need 17-th `char` for the `'\0'`, otherwise you'll get junk at the end of the string. – Sergey Kalinichenko May 26 '22 at 17:50
  • You ask about something not happening. Especially in that case, but also generally, you need to show a [mre]. – Yunnosch May 26 '22 at 17:50
  • okay @SergeyKalinichenko and M Oehm thanks, i got rid of the junky stuff, didnt notice i had one char missing.. Btw, my code STILL put some weird stuff instead of capitalizing the letter. Everything else work fine output is now: `Some !tring !ere` – al366io May 26 '22 at 17:51
  • 1
    `str2[i + 1] = toupper (str[i] + 1);` is incorrect. Should be `str2[i + 1] = toupper (str[i + 1]);` – debido May 26 '22 at 17:52
  • @Yunnosch if you copy paste my code into every sort of online editor it is itself a minimal reproducible example – al366io May 26 '22 at 17:53
  • It would take two copying steps and also some additional editing to get it to compile. So... no. – Yunnosch May 26 '22 at 17:54
  • This is [why you should always enable compiler warnings](https://stackoverflow.com/questions/57842756/why-should-i-always-enable-compiler-warnings). – einpoklum May 26 '22 at 22:51

2 Answers2

1

For starters this array

char stringa[16] = "some string here";

does not contain a string because it does not have a space to accommodate the terminating zero character '\0' of the string literal used as an initializer.

It would be better to declare it the following way without explicit specifying its size

char stringa[] = "some string here";

So this while loop

while (str[i])

can invoke undefined behavior.

Also you are using the uninitialized pointer str2.

char *str2;
//...
str2[i] = toupper (str[0]);

that again invokes undefined behavior.

Pay attention to that a passed string can contain more than one space between words and moreover can contain leading and trailing spaces. So this if statement also can invoke undefined behavior due to skipping the terminating zero character '\0' of the source string

    if (str[i] == ' ')
    {

      str2[i] = str[i];
      str2[i + 1] = toupper (str[i] + 1);
      i += 2;
    }

Hence your approach is in general wrong.

Such a function should return the modified source string.

Instead of the for loop it is better to use standard C functions strspn and strcspn.

Here is a demonstration program.

#include <string.h>
#include <stdio.h>
#include <ctype.h>

char * LetterCapitalize( char *s )
{
    const char *delim = " \t";

    for (char *p = s; *p; p += strcspn( p, delim ) )
    {
        p += strspn( p, delim );

        if (*p) *p = toupper( ( unsigned char )*p );
    }

    return s;
}

int main( void )
{
    char stringa[] = "some string here";
    puts( stringa );
    puts( LetterCapitalize( stringa ) );
}

The program output is

some string here
Some String Here
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
1

There are multiple problems:

  • The string defined in main is not null terminated because the initializer has exactly 16 characters, the defined length of the array, so there is no space for the null terminator. It is safer to omit the array length and let the compiler compute it from the initializer, including the null terminator:
   char stringa[] = "some string here";  // sizeof stringa == 17
  • str2 is uninitialized: storing characters to it has undefined behavior. You could instead either modify the argument string in place or allocate a copy and modify that.

  • the logic in LetterCapitalize is risky: you assume that words are separated by a single space and that the string does not end with a space.

  • the char argument to toupper() should be cast as (unsigned char) to avoid undefined behavior on negative char values on platforms where the type char is signed by default.

Here is a modified version:

#include <ctype.h>
#include <stdio.h>

char *LetterCapitalize(char *str) {
    unsigned char c, last = ' ';
    // uppercase characters that follow a space or at the start of the string
    for (size_t i = 0; (c = str[i]) != '\0'; last = c, i++) {
        if (last == ' ')
            str[i] = toupper(c);
    }
    return str;
}

int main() {
    char stringa[] = "some string here";
    puts(LetterCapitalize(stringa));
    return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189