patterncModerate
Simple string inverter program
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:
Since you don't modify the contents of the
Too many
You execute
As a quick solution you could just create a variable
Comparison unused
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.
const-modifierSince 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() callsYou 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.