Simple string inverter program











up vote
14
down vote

favorite
1












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;
}









share|improve this question
























  • I'm not sure about C but I know some languages (VB, C#, Swift) have string methods that allow you to invert your string.
    – TechnicalTophat
    Jan 23 '16 at 16:37












  • @SyntaxBit There is no such function in the standard C library.
    – 200_success
    Jan 24 '16 at 4:36










  • NEVER use gets() as it will allows the user to overrun the input buffer. It has been depreciated for a while and completely removed in the latest C standard. Suggest using fgets( string, sizeof(string), stdin );
    – user3629249
    Jan 25 '16 at 2:03










  • when #define'ing numeric values, always surround the value with parens to avoid any 'text replacement' errors
    – user3629249
    Jan 25 '16 at 2:05










  • note: strlen() actually returns a size_t value, not a int value. To avoid the implicit conversions, use size_t`
    – user3629249
    Jan 25 '16 at 2:06















up vote
14
down vote

favorite
1












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;
}









share|improve this question
























  • I'm not sure about C but I know some languages (VB, C#, Swift) have string methods that allow you to invert your string.
    – TechnicalTophat
    Jan 23 '16 at 16:37












  • @SyntaxBit There is no such function in the standard C library.
    – 200_success
    Jan 24 '16 at 4:36










  • NEVER use gets() as it will allows the user to overrun the input buffer. It has been depreciated for a while and completely removed in the latest C standard. Suggest using fgets( string, sizeof(string), stdin );
    – user3629249
    Jan 25 '16 at 2:03










  • when #define'ing numeric values, always surround the value with parens to avoid any 'text replacement' errors
    – user3629249
    Jan 25 '16 at 2:05










  • note: strlen() actually returns a size_t value, not a int value. To avoid the implicit conversions, use size_t`
    – user3629249
    Jan 25 '16 at 2:06













up vote
14
down vote

favorite
1









up vote
14
down vote

favorite
1






1





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;
}









share|improve this 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;
}






beginner c strings






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Jan 22 '16 at 18:31









Jamal

30.2k11115226




30.2k11115226










asked Jan 22 '16 at 17:16









Lúcio Cardoso

12819




12819












  • I'm not sure about C but I know some languages (VB, C#, Swift) have string methods that allow you to invert your string.
    – TechnicalTophat
    Jan 23 '16 at 16:37












  • @SyntaxBit There is no such function in the standard C library.
    – 200_success
    Jan 24 '16 at 4:36










  • NEVER use gets() as it will allows the user to overrun the input buffer. It has been depreciated for a while and completely removed in the latest C standard. Suggest using fgets( string, sizeof(string), stdin );
    – user3629249
    Jan 25 '16 at 2:03










  • when #define'ing numeric values, always surround the value with parens to avoid any 'text replacement' errors
    – user3629249
    Jan 25 '16 at 2:05










  • note: strlen() actually returns a size_t value, not a int value. To avoid the implicit conversions, use size_t`
    – user3629249
    Jan 25 '16 at 2:06


















  • I'm not sure about C but I know some languages (VB, C#, Swift) have string methods that allow you to invert your string.
    – TechnicalTophat
    Jan 23 '16 at 16:37












  • @SyntaxBit There is no such function in the standard C library.
    – 200_success
    Jan 24 '16 at 4:36










  • NEVER use gets() as it will allows the user to overrun the input buffer. It has been depreciated for a while and completely removed in the latest C standard. Suggest using fgets( string, sizeof(string), stdin );
    – user3629249
    Jan 25 '16 at 2:03










  • when #define'ing numeric values, always surround the value with parens to avoid any 'text replacement' errors
    – user3629249
    Jan 25 '16 at 2:05










  • note: strlen() actually returns a size_t value, not a int value. To avoid the implicit conversions, use size_t`
    – user3629249
    Jan 25 '16 at 2:06
















I'm not sure about C but I know some languages (VB, C#, Swift) have string methods that allow you to invert your string.
– TechnicalTophat
Jan 23 '16 at 16:37






I'm not sure about C but I know some languages (VB, C#, Swift) have string methods that allow you to invert your string.
– TechnicalTophat
Jan 23 '16 at 16:37














@SyntaxBit There is no such function in the standard C library.
– 200_success
Jan 24 '16 at 4:36




@SyntaxBit There is no such function in the standard C library.
– 200_success
Jan 24 '16 at 4:36












NEVER use gets() as it will allows the user to overrun the input buffer. It has been depreciated for a while and completely removed in the latest C standard. Suggest using fgets( string, sizeof(string), stdin );
– user3629249
Jan 25 '16 at 2:03




NEVER use gets() as it will allows the user to overrun the input buffer. It has been depreciated for a while and completely removed in the latest C standard. Suggest using fgets( string, sizeof(string), stdin );
– user3629249
Jan 25 '16 at 2:03












when #define'ing numeric values, always surround the value with parens to avoid any 'text replacement' errors
– user3629249
Jan 25 '16 at 2:05




when #define'ing numeric values, always surround the value with parens to avoid any 'text replacement' errors
– user3629249
Jan 25 '16 at 2:05












note: strlen() actually returns a size_t value, not a int value. To avoid the implicit conversions, use size_t`
– user3629249
Jan 25 '16 at 2:06




note: strlen() actually returns a size_t value, not a int value. To avoid the implicit conversions, use size_t`
– user3629249
Jan 25 '16 at 2:06










8 Answers
8






active

oldest

votes

















up vote
10
down vote



accepted










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.






share|improve this answer



















  • 3




    There are ways. You should look into memory-management. This is a quiet important topic for C-programmers, but you'll get to it in a little while. My best tip: Read K&R: The C-Programming Language - 2nd Edition. It's out there as a PDF for free and contains a whole lot of exercises and explains the most important features of C very well. Keep up the good stuff.
    – LastSecondsToLive
    Jan 22 '16 at 18:26




















up vote
10
down vote













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);





share|improve this answer



















  • 2




    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.
    – 200_success
    Jan 24 '16 at 4:35










  • @200_success That makes sense. I've referenced you in my post and updated it to have a bit more information on it (however, I did not remove the original part for reference (or, should I just remove it?)). Thanks!
    – SirPython
    Jan 24 '16 at 18:52


















up vote
7
down vote















  • 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 (''), 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.








share|improve this answer























  • If using puts(), be sure to NUL-terminate the string first.
    – 200_success
    Jan 23 '16 at 0:23










  • @200_success: True, but the break condition of the loop in question is based on a comparison with the result of strlen() anyway.
    – David Foerster
    Jan 23 '16 at 0:31




















up vote
5
down vote













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).






share|improve this answer





















  • Thanks a lot bro! Well, I would have used fgets, the only reason I didn't, is that I haven't learned it yet. Now, I'll certainly use it!
    – Lúcio Cardoso
    Jan 22 '16 at 21:18




















up vote
5
down vote













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






share|improve this answer





















  • If you null terminate it, then you can use printf("%s"... instead of using another for loop.
    – Jesse Weigert
    Jan 23 '16 at 5:31


















up vote
5
down vote













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 '' 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: %sn", string);
//prints out what you stored in string

printf("A single character: %cn", 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.






share|improve this answer






























    up vote
    4
    down vote













    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





    share|improve this answer





















    • Thanks a lot! Also, forgive for my English mistakes, it's just that i'm Brazilian, so, you know.
      – Lúcio Cardoso
      Jan 22 '16 at 18:15










    • @LúcioCardoso Glad to help! Your english was fine. If you mean the edit I made to your question, that's just a bit of tidying that people often do on the site. It's nothing to worry about!
      – SuperBiasedMan
      Jan 22 '16 at 18:47


















    up vote
    4
    down vote













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



    char my_string = {'a','b','c',''}; 


    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.






    share|improve this answer






















      protected by Community Apr 22 '16 at 3:00



      Thank you for your interest in this question.
      Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).



      Would you like to answer one of these unanswered questions instead?














      8 Answers
      8






      active

      oldest

      votes








      8 Answers
      8






      active

      oldest

      votes









      active

      oldest

      votes






      active

      oldest

      votes








      up vote
      10
      down vote



      accepted










      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.






      share|improve this answer



















      • 3




        There are ways. You should look into memory-management. This is a quiet important topic for C-programmers, but you'll get to it in a little while. My best tip: Read K&R: The C-Programming Language - 2nd Edition. It's out there as a PDF for free and contains a whole lot of exercises and explains the most important features of C very well. Keep up the good stuff.
        – LastSecondsToLive
        Jan 22 '16 at 18:26

















      up vote
      10
      down vote



      accepted










      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.






      share|improve this answer



















      • 3




        There are ways. You should look into memory-management. This is a quiet important topic for C-programmers, but you'll get to it in a little while. My best tip: Read K&R: The C-Programming Language - 2nd Edition. It's out there as a PDF for free and contains a whole lot of exercises and explains the most important features of C very well. Keep up the good stuff.
        – LastSecondsToLive
        Jan 22 '16 at 18:26















      up vote
      10
      down vote



      accepted







      up vote
      10
      down vote



      accepted






      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.






      share|improve this answer














      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.







      share|improve this answer














      share|improve this answer



      share|improve this answer








      edited Jan 22 '16 at 18:28

























      answered Jan 22 '16 at 18:15









      LastSecondsToLive

      477211




      477211








      • 3




        There are ways. You should look into memory-management. This is a quiet important topic for C-programmers, but you'll get to it in a little while. My best tip: Read K&R: The C-Programming Language - 2nd Edition. It's out there as a PDF for free and contains a whole lot of exercises and explains the most important features of C very well. Keep up the good stuff.
        – LastSecondsToLive
        Jan 22 '16 at 18:26
















      • 3




        There are ways. You should look into memory-management. This is a quiet important topic for C-programmers, but you'll get to it in a little while. My best tip: Read K&R: The C-Programming Language - 2nd Edition. It's out there as a PDF for free and contains a whole lot of exercises and explains the most important features of C very well. Keep up the good stuff.
        – LastSecondsToLive
        Jan 22 '16 at 18:26










      3




      3




      There are ways. You should look into memory-management. This is a quiet important topic for C-programmers, but you'll get to it in a little while. My best tip: Read K&R: The C-Programming Language - 2nd Edition. It's out there as a PDF for free and contains a whole lot of exercises and explains the most important features of C very well. Keep up the good stuff.
      – LastSecondsToLive
      Jan 22 '16 at 18:26






      There are ways. You should look into memory-management. This is a quiet important topic for C-programmers, but you'll get to it in a little while. My best tip: Read K&R: The C-Programming Language - 2nd Edition. It's out there as a PDF for free and contains a whole lot of exercises and explains the most important features of C very well. Keep up the good stuff.
      – LastSecondsToLive
      Jan 22 '16 at 18:26














      up vote
      10
      down vote













      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);





      share|improve this answer



















      • 2




        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.
        – 200_success
        Jan 24 '16 at 4:35










      • @200_success That makes sense. I've referenced you in my post and updated it to have a bit more information on it (however, I did not remove the original part for reference (or, should I just remove it?)). Thanks!
        – SirPython
        Jan 24 '16 at 18:52















      up vote
      10
      down vote













      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);





      share|improve this answer



















      • 2




        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.
        – 200_success
        Jan 24 '16 at 4:35










      • @200_success That makes sense. I've referenced you in my post and updated it to have a bit more information on it (however, I did not remove the original part for reference (or, should I just remove it?)). Thanks!
        – SirPython
        Jan 24 '16 at 18:52













      up vote
      10
      down vote










      up vote
      10
      down vote









      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);





      share|improve this answer














      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);






      share|improve this answer














      share|improve this answer



      share|improve this answer








      edited Jan 24 '16 at 18:51

























      answered Jan 22 '16 at 20:54









      SirPython

      11.9k32890




      11.9k32890








      • 2




        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.
        – 200_success
        Jan 24 '16 at 4:35










      • @200_success That makes sense. I've referenced you in my post and updated it to have a bit more information on it (however, I did not remove the original part for reference (or, should I just remove it?)). Thanks!
        – SirPython
        Jan 24 '16 at 18:52














      • 2




        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.
        – 200_success
        Jan 24 '16 at 4:35










      • @200_success That makes sense. I've referenced you in my post and updated it to have a bit more information on it (however, I did not remove the original part for reference (or, should I just remove it?)). Thanks!
        – SirPython
        Jan 24 '16 at 18:52








      2




      2




      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.
      – 200_success
      Jan 24 '16 at 4:35




      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.
      – 200_success
      Jan 24 '16 at 4:35












      @200_success That makes sense. I've referenced you in my post and updated it to have a bit more information on it (however, I did not remove the original part for reference (or, should I just remove it?)). Thanks!
      – SirPython
      Jan 24 '16 at 18:52




      @200_success That makes sense. I've referenced you in my post and updated it to have a bit more information on it (however, I did not remove the original part for reference (or, should I just remove it?)). Thanks!
      – SirPython
      Jan 24 '16 at 18:52










      up vote
      7
      down vote















      • 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 (''), 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.








      share|improve this answer























      • If using puts(), be sure to NUL-terminate the string first.
        – 200_success
        Jan 23 '16 at 0:23










      • @200_success: True, but the break condition of the loop in question is based on a comparison with the result of strlen() anyway.
        – David Foerster
        Jan 23 '16 at 0:31

















      up vote
      7
      down vote















      • 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 (''), 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.








      share|improve this answer























      • If using puts(), be sure to NUL-terminate the string first.
        – 200_success
        Jan 23 '16 at 0:23










      • @200_success: True, but the break condition of the loop in question is based on a comparison with the result of strlen() anyway.
        – David Foerster
        Jan 23 '16 at 0:31















      up vote
      7
      down vote










      up vote
      7
      down vote











      • 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 (''), 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.








      share|improve this answer
















      • 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 (''), 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.









      share|improve this answer














      share|improve this answer



      share|improve this answer








      edited Jan 29 '16 at 10:37

























      answered Jan 23 '16 at 0:18









      David Foerster

      1,600317




      1,600317












      • If using puts(), be sure to NUL-terminate the string first.
        – 200_success
        Jan 23 '16 at 0:23










      • @200_success: True, but the break condition of the loop in question is based on a comparison with the result of strlen() anyway.
        – David Foerster
        Jan 23 '16 at 0:31




















      • If using puts(), be sure to NUL-terminate the string first.
        – 200_success
        Jan 23 '16 at 0:23










      • @200_success: True, but the break condition of the loop in question is based on a comparison with the result of strlen() anyway.
        – David Foerster
        Jan 23 '16 at 0:31


















      If using puts(), be sure to NUL-terminate the string first.
      – 200_success
      Jan 23 '16 at 0:23




      If using puts(), be sure to NUL-terminate the string first.
      – 200_success
      Jan 23 '16 at 0:23












      @200_success: True, but the break condition of the loop in question is based on a comparison with the result of strlen() anyway.
      – David Foerster
      Jan 23 '16 at 0:31






      @200_success: True, but the break condition of the loop in question is based on a comparison with the result of strlen() anyway.
      – David Foerster
      Jan 23 '16 at 0:31












      up vote
      5
      down vote













      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).






      share|improve this answer





















      • Thanks a lot bro! Well, I would have used fgets, the only reason I didn't, is that I haven't learned it yet. Now, I'll certainly use it!
        – Lúcio Cardoso
        Jan 22 '16 at 21:18

















      up vote
      5
      down vote













      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).






      share|improve this answer





















      • Thanks a lot bro! Well, I would have used fgets, the only reason I didn't, is that I haven't learned it yet. Now, I'll certainly use it!
        – Lúcio Cardoso
        Jan 22 '16 at 21:18















      up vote
      5
      down vote










      up vote
      5
      down vote









      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).






      share|improve this answer












      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).







      share|improve this answer












      share|improve this answer



      share|improve this answer










      answered Jan 22 '16 at 20:16









      200_success

      128k15149412




      128k15149412












      • Thanks a lot bro! Well, I would have used fgets, the only reason I didn't, is that I haven't learned it yet. Now, I'll certainly use it!
        – Lúcio Cardoso
        Jan 22 '16 at 21:18




















      • Thanks a lot bro! Well, I would have used fgets, the only reason I didn't, is that I haven't learned it yet. Now, I'll certainly use it!
        – Lúcio Cardoso
        Jan 22 '16 at 21:18


















      Thanks a lot bro! Well, I would have used fgets, the only reason I didn't, is that I haven't learned it yet. Now, I'll certainly use it!
      – Lúcio Cardoso
      Jan 22 '16 at 21:18






      Thanks a lot bro! Well, I would have used fgets, the only reason I didn't, is that I haven't learned it yet. Now, I'll certainly use it!
      – Lúcio Cardoso
      Jan 22 '16 at 21:18












      up vote
      5
      down vote













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






      share|improve this answer





















      • If you null terminate it, then you can use printf("%s"... instead of using another for loop.
        – Jesse Weigert
        Jan 23 '16 at 5:31















      up vote
      5
      down vote













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






      share|improve this answer





















      • If you null terminate it, then you can use printf("%s"... instead of using another for loop.
        – Jesse Weigert
        Jan 23 '16 at 5:31













      up vote
      5
      down vote










      up vote
      5
      down vote









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






      share|improve this answer












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







      share|improve this answer












      share|improve this answer



      share|improve this answer










      answered Jan 23 '16 at 1:05









      Jesse Weigert

      1512




      1512












      • If you null terminate it, then you can use printf("%s"... instead of using another for loop.
        – Jesse Weigert
        Jan 23 '16 at 5:31


















      • If you null terminate it, then you can use printf("%s"... instead of using another for loop.
        – Jesse Weigert
        Jan 23 '16 at 5:31
















      If you null terminate it, then you can use printf("%s"... instead of using another for loop.
      – Jesse Weigert
      Jan 23 '16 at 5:31




      If you null terminate it, then you can use printf("%s"... instead of using another for loop.
      – Jesse Weigert
      Jan 23 '16 at 5:31










      up vote
      5
      down vote













      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 '' 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: %sn", string);
      //prints out what you stored in string

      printf("A single character: %cn", 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.






      share|improve this answer



























        up vote
        5
        down vote













        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 '' 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: %sn", string);
        //prints out what you stored in string

        printf("A single character: %cn", 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.






        share|improve this answer

























          up vote
          5
          down vote










          up vote
          5
          down vote









          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 '' 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: %sn", string);
          //prints out what you stored in string

          printf("A single character: %cn", 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.






          share|improve this answer














          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 '' 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: %sn", string);
          //prints out what you stored in string

          printf("A single character: %cn", 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.







          share|improve this answer














          share|improve this answer



          share|improve this answer








          edited Jan 24 '16 at 0:06









          Jamal

          30.2k11115226




          30.2k11115226










          answered Jan 23 '16 at 23:59









          Spoofy McGee

          491




          491






















              up vote
              4
              down vote













              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





              share|improve this answer





















              • Thanks a lot! Also, forgive for my English mistakes, it's just that i'm Brazilian, so, you know.
                – Lúcio Cardoso
                Jan 22 '16 at 18:15










              • @LúcioCardoso Glad to help! Your english was fine. If you mean the edit I made to your question, that's just a bit of tidying that people often do on the site. It's nothing to worry about!
                – SuperBiasedMan
                Jan 22 '16 at 18:47















              up vote
              4
              down vote













              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





              share|improve this answer





















              • Thanks a lot! Also, forgive for my English mistakes, it's just that i'm Brazilian, so, you know.
                – Lúcio Cardoso
                Jan 22 '16 at 18:15










              • @LúcioCardoso Glad to help! Your english was fine. If you mean the edit I made to your question, that's just a bit of tidying that people often do on the site. It's nothing to worry about!
                – SuperBiasedMan
                Jan 22 '16 at 18:47













              up vote
              4
              down vote










              up vote
              4
              down vote









              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





              share|improve this answer












              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






              share|improve this answer












              share|improve this answer



              share|improve this answer










              answered Jan 22 '16 at 17:48









              SuperBiasedMan

              11.8k52660




              11.8k52660












              • Thanks a lot! Also, forgive for my English mistakes, it's just that i'm Brazilian, so, you know.
                – Lúcio Cardoso
                Jan 22 '16 at 18:15










              • @LúcioCardoso Glad to help! Your english was fine. If you mean the edit I made to your question, that's just a bit of tidying that people often do on the site. It's nothing to worry about!
                – SuperBiasedMan
                Jan 22 '16 at 18:47


















              • Thanks a lot! Also, forgive for my English mistakes, it's just that i'm Brazilian, so, you know.
                – Lúcio Cardoso
                Jan 22 '16 at 18:15










              • @LúcioCardoso Glad to help! Your english was fine. If you mean the edit I made to your question, that's just a bit of tidying that people often do on the site. It's nothing to worry about!
                – SuperBiasedMan
                Jan 22 '16 at 18:47
















              Thanks a lot! Also, forgive for my English mistakes, it's just that i'm Brazilian, so, you know.
              – Lúcio Cardoso
              Jan 22 '16 at 18:15




              Thanks a lot! Also, forgive for my English mistakes, it's just that i'm Brazilian, so, you know.
              – Lúcio Cardoso
              Jan 22 '16 at 18:15












              @LúcioCardoso Glad to help! Your english was fine. If you mean the edit I made to your question, that's just a bit of tidying that people often do on the site. It's nothing to worry about!
              – SuperBiasedMan
              Jan 22 '16 at 18:47




              @LúcioCardoso Glad to help! Your english was fine. If you mean the edit I made to your question, that's just a bit of tidying that people often do on the site. It's nothing to worry about!
              – SuperBiasedMan
              Jan 22 '16 at 18:47










              up vote
              4
              down vote













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



              char my_string = {'a','b','c',''}; 


              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.






              share|improve this answer



























                up vote
                4
                down vote













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



                char my_string = {'a','b','c',''}; 


                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.






                share|improve this answer

























                  up vote
                  4
                  down vote










                  up vote
                  4
                  down vote









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



                  char my_string = {'a','b','c',''}; 


                  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.






                  share|improve this answer














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



                  char my_string = {'a','b','c',''}; 


                  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.







                  share|improve this answer














                  share|improve this answer



                  share|improve this answer








                  edited 1 hour ago









                  albert

                  1371




                  1371










                  answered Jan 23 '16 at 9:59









                  fhtuft

                  1414




                  1414

















                      protected by Community Apr 22 '16 at 3:00



                      Thank you for your interest in this question.
                      Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).



                      Would you like to answer one of these unanswered questions instead?



                      Popular posts from this blog

                      Ellipse (mathématiques)

                      Quarter-circle Tiles

                      Mont Emei