HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMajor

C program that reverses a string

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
stringprogramreversesthat

Problem

As practice with the (ever so painful) C Strings I wrote a string reversal program. I am new to C, so I wanted to make sure that I am not using bad coding practices. It runs just fine (at least with expected input), but I am not sure if the logic is as good as it could be.

#include 
#include 
#include 
char* strreverse(char* c) 
{
    char input[500], output[500] = {}, buffer[2] = {' ', '\0'};
    strcpy(input, c);
    int i;
    for(i = strlen(input); i >= 0; i--)
    {
        buffer[0] = input[i];
        strcat(output, buffer);
    }
    return (char*) output;
}

int main(void)
{
    char* in = malloc(256);
    printf("Enter a string: ");
    scanf("%s", in);
    char* out = strreverse(in);
    printf("%s", out);
    return 0; 
}

Solution

Implementation:

As of now, the code is not practical, because the limit is 500 chars including zero termination. It does unnecessary copying. You need to determine the actual length of the string by relying on the fact that C strings are null terminated.

size_t length = 0;
while (*(str + length) != 0)
{
    ++length;
}


Notice that I'm using size_t, because it is meant to be used to describe size of the objects in memory. Despite it is easy to do it manually, you should always use strlen when possible:

size_t length = strlen(str);


It should be noted that the function returns the length without null terminator. Then we allocate memory for result, since we are creating new string, rather than making changes in place:

char* result = malloc(length + 1);
if (result == NULL)
{
    return NULL;
}


We check for succession of allocation, as was suggested in the comments. If allocation failed, further operations will definitely invoke undefined behavior, so we return NULL. length + 1 is for null terminator. Then, we immediately null terminate it:

result[length] = 0;


Now we need to capture the position of the last character in the original string, to start copying from there. It is right before null terminator, which is on index length, so needed position is length - 1. There is an edge case of length 0, so we should check for that first

if (length == 0)
{
    return result;
}
size_t last = length - 1;


Then we write algorithm that writes contents of the first string into the second in reverse order:

size_t it = 0;
while (it <= last)
{
    result[it] = str[last - it];
    ++it;
}


then simply return the result:

return result;


Although the implementation has good performance, there are some opportunities to make micro optimizations. The function input type should be const char*, since we don't modify original string. Additionally, it should be noted that the caller is responsible for freeing up malloc'd memory.

Cosmetics:

Name of the function is a little hard to read, so I'd suggest str_reverse. Also, c is not good name at all, so it would be better if it's name str

Put together:

#include 
#include 

char* str_reverse(const char* str)
{
    size_t length = strlen(str);

    char* result = malloc(length + 1);
    if (result == NULL)
    {
        return NULL;
    }

    result[length] = 0;

    if (length == 0)
    {
        return result;
    }

    size_t last = length - 1;
    size_t it = 0;
    while (it <= last)
    {
        result[it] = str[last - it];
        ++it;
    }

    return result;
}

int main(void) {
    const char* str = "ABCD";
    char* reversed = str_reverse(str);

    printf("%s\n", reversed);
    if (str[4] != 0)
    {
        printf("problems\n");
    }

    free(reversed);
    return 0;
}

Code Snippets

size_t length = 0;
while (*(str + length) != 0)
{
    ++length;
}
size_t length = strlen(str);
char* result = malloc(length + 1);
if (result == NULL)
{
    return NULL;
}
result[length] = 0;
if (length == 0)
{
    return result;
}
size_t last = length - 1;

Context

StackExchange Code Review Q#134490, answer score: 43

Revisions (0)

No revisions yet.