Why is the new string not being printed after adding char characters?

Question

I'm stating to write some leetspeak code in C, however, I keep getting error like:

error: incompatible integer to pointer conversion returning 'char' from a function with result type 'string' (aka 'char *') [-werror,-wint-conversion]

or simply my latest code is not returning any value.

#include <cs50.h>
#include <stdio.h>

string replace(string no_vowels);

int main(int argc, string argv[])
{
    if (argc != 2)
    {
        printf("Error message\n");
        return 1;
    }
    else
    {
        printf("This is your message: %s\n", replace(argv[1]));
    }
}

string replace(string no_vowels)
{
    int i = 0;
    while (no_vowels[i] != '\0')
    {
        i++;
    }

    string subs = "6310";
    string new_string = "";

    for (int l = 0; l < i; l++)
    {
        if (no_vowels[l] == 'a')
        {
            new_string += subs[0];
        }
        else if (no_vowels[l] == 'e')
        {
            new_string += subs[1];
        }
        else if (no_vowels[l] == 'i')
        {
            new_string += subs[2];
        }
        else if (no_vowels[l] == 'o')
        {
            new_string += subs[3];
        }
        new_string += no_vowels[l];
    }
    return new_string;
}

I tried creating arrays with the numbers that should substitute the letters directly, tried to add or subtract values taking in consideration each char character on the ASCII table and looked for existing threads on StackOverflow

PS: I am trying to do this exercise with the libraries that are already imported.

Answer 1

You cannot concatenate strings in C using += as you would in javascript. For this assignment, you could modify the argument string in place and return the original pointer.

Here is a modified version:

string replace(string no_vowels)
{
    string subs = "6310";

    for (int i = 0; no_vowels[i] != '\0'; i++)
    {
        if (no_vowels[i] == 'a')
        {
            no_vowels[i] = subs[0];
        }
        else if (no_vowels[i] == 'e')
        {
            no_vowels[i] = subs[1];
        }
        else if (no_vowels[i] == 'i')
        {
            no_vowels[i] = subs[2];
        }
        else if (no_vowels[i] == 'o')
        {
            no_vowels[i] = subs[3];
        }
    }
    return no_vowels;
}

Note also that the purpose of the string typedef in the CS50 course is to hide the implementation and avoid pointer shock for C newbies. It is like trailing wheels for small kids, you should very quickly remove them and learn the real skills.

Here is a modified version with the actual types:

#include <cs50.h>
#include <stdio.h>

char *replace(char *str);

int main(int argc, char *argv[]) {
    if (argc != 2) {
        printf("Invalid argument count\n");
        return 1;
    } else {
        printf("This is your message: %s\n", replace(argv[1]));
        return 0;
    }
}

// replace vowels in place in the argument string
char *replace(char *str) {
    const char *subs = "6310";

    for (int i = 0; str[i] != '\0'; i++) {
        if (str[i] == 'a') {
            str[i] = subs[0];
        } else
        if (str[i] == 'e') {
            str[i] = subs[1];
        } else
        if (str[i] == 'i') {
            str[i] = subs[2];
        } else
        if (str[i] == 'o') {
            str[i] = subs[3];
        }
    }
    return str;
}

Here are alternative implementations you can study:

Using a switch statement:

char *replace(char *str) {
    const char *subs = "6310";

    for (int i = 0; str[i] != '\0'; i++) {
        switch (str[i]) {
          case 'a':
            str[i] = subs[0];
            break;
          case 'e':
            str[i] = subs[1];
            break;
          case 'i':
            str[i] = subs[2];
            break;
          case 'o':
            str[i] = subs[3];
            break;
        }
    }
    return str;
}

Using memchr from <string.h>:

#include <string.h>

char *replace(char *str) {
    for (int i = 0; str[i] != '\0'; i++) {
        const char *p = memchr("aeio6310", str[i], 4);
        if (p != NULL) {
            str[i] = p[4];
        }
    }
    return str;
}

Answer 2

I'm going to assume based on your code that you're not meant to modify the input string, but rather return a new string with the modified contents.

C strings are lower level than strings in something like Python. A C string is an array of characters then length of the string plus one for null terminator ('\0').

You need to create such an array for new_string. Fortunately, you know the length, since your program doesn't change the length of the original string. You can allocate a new string, copy into it following your replacement logic, then return the new string.

A first naive attempt at this might look like:

char * replace(const char *str) {
    size_t len = strlen(str);
    char new_string[len + 1];

    for (size_t i = 0; i < len + 1; i++) {
        // assign new_string[i] from str[i] as appropriate
    }

    // Assign the null terminator.
    new_string[len] = '\0';

    // Returning a pointer to the local variable,
    // see note below about "undefined behavior."
    return new_string;
} 

Except C has another gotcha. The pointer you're returning might still point to that array, or it might not. By returning a pointer to a local variable from a function, you invoke undefined behavior. Keep in mind that undefined behavior does not necessarily mean your program will fail. It means it could fail. It could continue but do something you didn't expect. It could also work the way you expect. This is especially dangerous as it can lead you to have unrealistic expectations for what comprises "good code."

To avoid this, you need to dynamically allocate that new string so that the memory "outlives" the function call. You'll also want to check the return of malloc before using that memory, in case it didn't work and didn't allocate you the memory you were expecting.

char * replace(const char *str) {
    size_t len = strlen(str);
    char *new_string = malloc(len + 1);
    if (new_string == NULL) return NULL;

    for (size_t i = 0; i < len + 1; i++) {
        // assign new_string[i] from str[i] as appropriate
    }

    // Assign the null terminator.
    new_string[len] = '\0';

    return new_string;
} 

You should also remember to free the dynamically allocated memory in main. This means you need to hang onto a pointer to it, so you cannot use it directly in your call to printf. This also gives you an opportunity to determine that the memory allocation for the new string succeeded.

int main(int argc, char *argv[]) {
    if (argc != 2) {
        printf("Error message\n");
        return 1;
    }
    
    char *replacement = replace(argv[1]);
    
    if (replacement == NULL) {
        printf("Memory allocation failed\n");
        return 1;
    }

    printf("This is your message: %s\n", replacement);
    free(replacement);
}

Answer 3

Your function definition is invalid.

In this line

string new_string = "";

you declared a pointer to a string literal. Any attempt to change a string literal results in undefined behavior.

From the C Standard (6.4.5 String literals)

7 It is unspecified whether these arrays are distinct provided their elements have the appropriate values. If the program attempts to modify such an array, the behavior is undefined.

In statements like that

new_string += subs[0];

there is used the pointer arithmetic that changes the pointer new_string itself. As a result the pointer has an invalid value that does not point to a valid object declared in the function. And if to assume that such statements are rewritten correctly nevertheless this statement

new_string += no_vowels[l];

is in any case wrong because logically you are trying to overwrite already updated character in if-else statements.

This while loop

while (no_vowels[i] != '\0')
{
    i++;
}

is redundant. Strings are ended with the terminating zero character '\0'. So you can use this fact creating a condition of a loop for travesing a string. In any case the variables i and l should have the unsigned type size_t instead of the signed type int.

As far as I know the letter 'u' is also a vowel. However you are ignoring it together with upper case vowels.

There is no need to create a new string. According to the C Standard you may change program parameters pointed to ponters argv.

From the C Standard (5.1.2.2.1 Program startup)

2. — The parameters argc and argv and the strings pointed to by the argv array shall be modifiable by the program, and retain their last-stored values between program startup and program termination.

The function can look the following way

string replace( string no_vowels )
{
    string subs = "6310";

    for ( char *p = no_vowels; *p != '\0'; ++p )
    {
        if ( *p == 'a' )
        {
            *p = subs[0];
        }
        else if ( *p == 'e' )
        {
            *p = subs[1];
        }
        else if ( *p == 'i' )
        {
            *p = subs[2];
        }
        else if ( *p == 'o' )
        {
            *p = subs[3];
        }
    }

    return no_vowels;
}