C character array comparison

Question

Ok so I was doing a coding challenge on codewars.com and the challenge was to take a string as input and return a string where in place of the letters are instead the number of the alphabet that matches the letter.

Everything except letters are to be ignored.

ex: "aab" would return "1 1 2"

There should be a space between each number that represents a letter in the alphabet.

So, when I run this code on my IDE (which is xcode using c99) Everything looks good and the strcmp() function says the 2 strings are equal. The website I'm on uses C11 I believe but I don't see that causing the error.

When I run this code on the challenge website it passes a couple of tests but then fails a couple also. It fails when the input string is "", and it also fails on the string that I have used in the code below, but again it does not fail when I run it on my ide.

My questions are:

1) Any idea what is causing this bug?

2) What would you have done differently as far as the code is concerned

Thanks

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

char *alphabet_position(char *text)
{
    int i,letter_position;
    unsigned long int size = strlen(text);

    char *result = malloc(sizeof(int)*size + 1);

    char int_string[10];
    char temp = ' ';

    //If String wasn't blank
    if (strcmp(text, "")!=0)
    {
        for (i=0; i<size-1; i++)
        {
            //If it is a letter
            if (isalpha(text[i]))
            {
                temp = tolower(text[i]);
                if (temp == 'a')
                    strcat(result, "1");
                else
                {
                    letter_position = temp - 'a' + 1;
                    sprintf(int_string,"%d",letter_position);
                    strcat(result, int_string);
                }
                //Print space after letter until the last letter
                if (i!=size-2)
                    strcat(result, " ");
            }
        }
        strcat(result, "\0");
        return result;
    }
    else
    {
        strcat(result, "\0");
        return result;
    }

}


int main(void)
{

    char *string = alphabet_position("The narwhal bacons at midnight.");
    char *expected_output = "20 8 5 14 1 18 23 8 1 12 2 1 3 15 14 19 1 20 13 9 4 14 9 7 8 20";

    printf("Your output %s\n", alphabet_position("The narwhal bacons at midnight."));
    printf("Expt output %s\n", "20 8 5 14 1 18 23 8 1 12 2 1 3 15 14 19 1 20 13 9 4 14 9 7 8 20");

    printf("\n");

    printf("your len %lu\n", strlen(alphabet_position("The narwhal bacons at midnight.")));
    printf("Expt len %lu\n", strlen(expected_output));

    if (strcmp(string, expected_output)==0)
        printf("Equal\n");
    else
        printf("Not equal\n");
    return 0;
}

You never initialized the contents of result, so strcat(result, "1"); causes undefined behavior. — Barmar
– Barmar, Commented May 8, 2019 at 16:51
Why is your loop only up to size-1? You're ignoring the last character of the input string. — Barmar
– Barmar, Commented May 8, 2019 at 16:52
There's no need for strcat(result, "\0");. All the previous calls to strcat() will add a null terminator. — Barmar
– Barmar, Commented May 8, 2019 at 16:53
Why do you have a special case for temp == 'a'? Your general calculation temp - 'a' + 1 will work for it. — Barmar
– Barmar, Commented May 8, 2019 at 16:54
set result[0] = 0 after the malloc. This will make it a null terminated empty string that you can then pass to 'strcat' — MFisherKDX
– MFisherKDX, Commented May 8, 2019 at 17:04

Barmar · Accepted Answer · 2019-05-08 17:10:38Z

4

You have two serious problems.

First, you're not initializing the contents of the result array to an empty string. After you call malloc(), add:

result[0] = '\0';

malloc() doesn't initialize the memory it allocates. There's another function calloc() that takes slightly different arguments and initializes the memory to zeroes. But you only need the first character to be zero, so there's no need for that.

Second, the for loop is not processing the last character of text. It should be:

for (i = 0; i < size; i++)

Similarly, the test for whether to add a space should be if (i != size-1). Did you think strlen() counts the null character at the end?

The amount of space you specify in malloc() is not correct, but in practice it won't cause a problem. sizeof(int) has nothing to do with the number of characters it takes to show the value of an integer. Since you're just printing the alphabetical position, it will be at most 26, so you need 3 characters for every input character. Therefore, it should be:

char *result = malloc(3 * size + 1);

Your allocation works because sizeof(int) is generally at least 4, so you're allocating more than enough space.

There are other minor problems that don't affect the correctness of the result:

You don't need the if statement that treats 'a' specially, the code you have in else will work for all letters.

You don't need strcat(result, "\0") at the end. result has to already be null-terminated in order for you to use it as an argument to strcat(), so there's no point in using strcat() to add a null terminator.

edited May 8, 2019 at 17:10

answered May 8, 2019 at 17:04

Barmar

789k57 gold badges555 silver badges669 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

RobotMan Over a year ago

Thank you for taking the time to help, you cleared up some of the confusion I was having about initializing char arrays as well as some of the other unnecessary code that was in there. I had no idea strcat added a null terminator either lol.

user3386109 Over a year ago

I think this introduces a subtle bug when the input doesn't end with a letter. There will be an extra space at the end of the output string.

Barmar Over a year ago

Yeah. A simpler solution would be to put space after every number, and at the end overwrite the last space with a null byte.

Barmar Over a year ago

Another method would be to write the space before each number, unless it's the first number being printed.

Jasen Over a year ago

Your allocation works because sizeof(int) is generally at least 4, so you're allocating more than enough space., actually that explains why it fails on an empty string.

|

Collectives™ on Stack Overflow

C character array comparison

1 Answer 1

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related