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

Simple string inverter program

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

Problem

I am currently studying c programming, but I'm at a very basic level. So for practice I made a program that reads a string and inverts it. I want to know if the code is readable and how I can improve it.

/* 
* Goal: Given a string, invert it, then print the result.
* Author: Lúcio Cardoso
*/

#include 
#include 

/*This is the size of the arrays in this program. It's better to use a          
constant, because makes easier to modify the arrays' size, if needed.*/
#define NUM 200

void invert(char *s) //Fucntion responsible for inverting the string.
{
    char aux[NUM];
    int i, j, n; //These variables are all used to control the for loops.

    /*This loop will read the size of the array's string, that's why
    strlen(s) - 1 is used. We don't need to read all contents
    from the array, just the string. As we read the s[] backwards,
    its content is stored in aux[n]. In this case, from zero to the
    size of s[]'s string - 1. Minus 1 is used because we read it from ZERO,
    not i. */
    for (i = strlen(s) - 1, n = 0; n = 0; i--, n++) {
        aux[n] = s[i];
    }

    printf("Included with the super mojo from the string inverter, ");
    printf("this is the result: ");

    //This for loop reads all the content from the aux array, and print it.
    for (j = 0; j ");
    gets(string); //Reads the string.

    //Now, we only need to call the function with string[] as a parameter.
    invert(string);

    return 0;
}

Solution

That's a fine small piece of software for a beginner. Here is my review:

const-modifier

Since you don't modify the contents of the char s you can safely change your function signature to void invert(const char s).

Too many strlen() calls

You execute strlen(s) every for-loop iteration. This is quite bad for the performance (especially for large strings) since strlen loops through your whole string each call. If your string is 100 characters long (beside the NULL-character) this ends up in 100*100 = 10000 iterations.

As a quick solution you could just create a variable length and store the length of your string once. From that point on you'll compare with length instead of strlen(x) and will get the same result (since, s doesn't change while execution).

Comparison unused
n < strlen(s) remains unused in your first for-loop. I think you want to connect the two comparisons with &&:

for (i = strlen(s) - 1, n = 0; n = 0; i--, n++)


Why even reverse?

Since your function prints the reversed string, but doesn't return anything. You don't really need to reverse it. You could just loop through the input string starting at the end (like you did in your first loop), printing all the characters. The extra array only makes sense, if you return a pointer to it for later use.

Code Snippets

for (i = strlen(s) - 1, n = 0; n < strlen(s) && i >= 0; i--, n++)

Context

StackExchange Code Review Q#117608, answer score: 10

Revisions (0)

No revisions yet.