Simple string inverter program

Question

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 <stdio.h>
#include <string.h>

/*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 < strlen(s), i >= 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 < strlen(s); j++) {
        printf("%c", aux[j]);
    }
}

int main(void)
{
    char string[NUM];

    printf("\nWelcome to the super, duper string inverter!\n");
    printf("If you want to see some magic, enter a string:\n>");
    gets(string); //Reads the string.

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

    return 0;
}

Answer 1

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 < strlen(s) && i >= 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.

Answer 2

Usability

Your function isn't very flexible. If another piece of code needs to use it, it will have to use it for a single purpose, and that purpose is to be given a string and to print out that string in reverse order.

Below are some ways you could make this more flexible:

---

User-supplied output buffer

Right now, the reverse string is just printed straight to STDOUT. To make this more flexible and so that it follows the single responsibility principle, you could have the function accept an output buffer to stick the reversed string into:

void reverse(char *s, char *out) {

With this, you should also remove the printf calls in your function.

Note: a better alternative to this would be to overwrite the original string. Rather than accepting an output buffer, you can create a temporary output buffer in the function (like you are with aux). Then, simply copy the temporary output buffer into the input string at the end of the function. As 200_success states,

I do recommend treating s as an in-out parameter, and reversing the string in place. If the caller wants to keep a copy of the original, then they can duplicate it themselves first. Placing the responsibility of buffer allocation on the caller reduces memory-management headaches in C.

Basically, by having the output go to the input string, memory management becomes much less complex and is overall easier on the caller.

---

User-supplied length

What if the code using this function doesn't want to have the entire string reversed? What if they only want the first part of the string reversed? To allow for this, you function should accept the amount of characters to reverse:

void reverse(char *s, char *out, size_t len) {

Then, you simply use that len in your loops as you have been already.

---

Your code

Enough of that; time to review the code you have presented.

---

Printing out a string

for (j = 0; j < strlen(s); j++) {
    printf("%c", aux[j]);
}

Here, you are looping through the aux "array" and printing out each character. However, you are over-complicating it; why not just print aux?

printf("%s", aux);

Answer 3

• You're using printf to print a single char:

  printf("%c", aux[j]);
  

  printf is a very handy tool to format output, but it carries quite a bit of overhead with it – not something you want to call over and over without a good reason. A much more efficient way to print a single char to stdout would be:

  putchar(aux[j]);
  

  Or, instead of looping over the string to print it character-by-character, you can print the whole thing with just one function call:

  puts(aux);
  

  (Note that puts prints an additional newline character at the end of the string. If you want to avoid that, use fputs(aux, stdout) instead.)

  puts and fputs require the string (aux) to be terminated by a NUL-character ('\0'), which it isn't in your program. If you want to print a character sequence of known length (but possibly without such a terminator character), you can use:

  fwrite(aux, 1, strlen(n), stdout);
  

• You can also use puts or fputs to print all the string literals in your program with less overhead, e. g.

  puts("\nWelcome to the super, duper string inverter!");
  

  instead of

  printf("\nWelcome to the super, duper string inverter!\n");
  

  Now you don't even need to worry about % characters being interpreted as format descriptors.

• You can concatenate string literals that are printed right after each other to avoid the overhead of additional buffering and I/O locking and unlocking:

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

  If you don't like long string literals you can break them into multiple parts:

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

  produces exactly the same syntax tree and binary code as the source code just before.

Answer 4

Never, ever, use gets(). It is flawed by design: it does not limit the length of the input it accepts. A buffer overflow is possible if the input is too long, and gets() will let it happen. The function exists only for compatibility with old code.

Rather, you should use fgets(string, length, stdin) — in your case, fgets(string, NUM, stdin).

Answer 5

You need to ensure that your aux string is null terminated. Add the line aux[strlen(s)] = '\0'; right after the first for loop.

Answer 6

There are a number of ways to approach this problem, but I like doing it in one of two (maybe archaic but simple) ways. You did well using just arrays but I'll explain a good way to approach this using beginners C level functions.

One way to invert a string is to run a for loop (or while loop) just like you did. However, you should familiarize yourself with pointers so you don't have to run your second for loop to print out all of the characters individually. You're already implementing some pointer logic with your arrays, but I'll add some information that should be helpful.

Character Pointers and Strings

Say I want a string that holds 100 characters, but I want to make use of pointers instead of starting with an array. Simply create a char* and assign it a 100 slots of memory with malloc() (+1 slot for the null character which terminates the string). malloc stands for memory allocate.

char* string;
string = (char*)malloc(101*sizeof(char));
//char string[101] is the same as this code above

Here string holds 101 places in memory that are size char (8 bits). String can be used just like an array to look at any one character. If we now store a bunch of information in this area with fgets() or scanf(), we can access any one character just like you did with aux[j].

for (j = 0; j < strlen(s); j++) {

    printf("%c", aux[j]);

}

Printing an Entire String

However, if we want to print the entire string after assigning the values/characters, simply use %s instead of looping through the entire array for each character. Let's use my predefined string pointer.

printf("%s", string);

Here you need to make sure your string ends with a null character '\0' or this printf will print out unassigned space in memory converted to character form.

Inverting a String

Do as you just did, by assigning the last position of your initial string (s) to the first position of your inverted string (aux) and continue on throughout the array. You did this well.

Freeing Pointer Memory

Finally, don't forget to free the memory you used and don't need anymore when moving on in your programs. For this simple project, freeing the memory you allocated for the pointer shouldn't matter all that much but is good for you to practice. This works only for things you've used malloc() with. This will free up the space you reserved (once you free, you're not getting that data back).

free(string);

Simple pointer code:

char* string;
string = (char*)malloc(101*sizeof(char));
//creates a pointer and gives it 101 slots of memory size character

fgets(string,100,stdin);
//retrieves input of 100 through stdin (your input) and stores in string

printf("You entered: %s\n", string);
//prints out what you stored in string

printf("A single character: %c\n", string[5]);
//prints out what you stored in position 5 of string
//assuming your string made it to at least 5

free(string);
//clears out the memory locations you reserved for string

Further Pointer Information

Pointers are confusing for most to learn about at first. They just point to information, they don't know what it is or care about what happens to it. Think of a pointer as a guy whose job is to point at your wallet so you always know where it's at. The guy doesn't care about how much money is inside, he just tells you the location of the wallet whenever you ask him. De-referencing his information (going to the location yourself and looking at the value) will tell you how much money is inside your wallet.

You can have two pointers pointing to the same thing, or one pointer pointing at another pointer. A pointer can direct you toward 1 thing or have it point at many things by using malloc(). You can reuse the same char* after freeing it, if you plan to point to more than one thing you need to malloc() however many spaces. A pointer can be an int, char, float, etc. all predefined with a *.

int* a;
float* b;
char* c;
dog* d; //after you "typedef dog"
char**e; //used for creating a list of pointers or 2D arrays
int f[300][300]; //same as char** e but of type int;
//etc

//if you want to use a string for later use after freeing it, point to NULL
c = NULL;

When I was learning to better implement pointers, I watched a video by suckerpinch about adding words to one another and making a Portmanteau. If you want to better familiarize yourself with pointers, try making a simple version of this where you concatenate strings (adding strings to one another - possibly helpful function: strcat()), print strings out frequently, etc. If you can make something like this, you'll get a fairly good handle on strings/pointers.

Answer 7

You've done a lot of work to comment and clarify your code, but I think it's actually too much commenting. Sometimes that reads as a wall of text instead of helpful brief comments. For instance:

/*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

The first sentence is very verbose, while the rest is just a programming principle. If that's for your own reference as a beginner that's fine, but most programmers will know that. I'd suggest having no comment, and renaming your constant ARRAY_SIZE, as that you what you need to know anyway.

Comments are good, but most effective when short as they're more likely to be read that way. Usually people will know what the common functions of a program do, but comments can help illuminate what you're doing or why you need to do things a certain way. For your first for loop in invert, I'd keep it to a simple comment:

// Loop backwards over s and store it in aux

Answer 8

Remember that strings are null terminated char arrays. Like this:

char my_string[] = {'a','b','c','\0'}; 

I would have called NUM, MAX_STRING (or something like it). aux, should then be

char aux[MAX_STRING +1] //+1 for null terimination

The name aux, is also not good. Use descriptive names on variables, like string_inverted.

Also your function is invert a string and print it, not invert a string. Its better to let a function only do one thing, this let you use your functions as primitives for building other functions. The prototype could be like this.

void invert(const char *string,char * inverted_string);

Also if you use a function only inside one compilation unit (c file), make them static.

As other have pointed out, you comment a lot. An ideal code (that don't exist) should not need comments. Use comments to aide code reading when code deviate from this ideal. If you use comments for documenting code, which can be a good idea, specially if you work with others, look in to doxygen.