Simple string inverter program
up vote
14
down vote
favorite
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
|
show 1 more comment
up vote
14
down vote
favorite
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
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 usegets()
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 usingfgets( 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 asize_t
value, not aint
value. To avoid theimplicit conversions, use
size_t`
– user3629249
Jan 25 '16 at 2:06
|
show 1 more comment
up vote
14
down vote
favorite
up vote
14
down vote
favorite
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
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
beginner c strings
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 usegets()
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 usingfgets( 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 asize_t
value, not aint
value. To avoid theimplicit conversions, use
size_t`
– user3629249
Jan 25 '16 at 2:06
|
show 1 more comment
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 usegets()
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 usingfgets( 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 asize_t
value, not aint
value. To avoid theimplicit 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
|
show 1 more comment
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.
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
add a comment |
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);
2
I do recommend treatings
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
add a comment |
up vote
7
down vote
You're using
printf
to print a singlechar
:
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 singlechar
tostdout
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, usefputs(aux, stdout)
instead.)
puts
andfputs
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
orfputs
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.
If usingputs()
, 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 ofstrlen()
anyway.
– David Foerster
Jan 23 '16 at 0:31
add a comment |
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)
.
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
add a comment |
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.
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
add a comment |
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.
add a comment |
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
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
add a comment |
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.
add a comment |
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.
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
add a comment |
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.
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
add a comment |
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.
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.
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
add a comment |
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
add a comment |
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);
2
I do recommend treatings
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
add a comment |
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);
2
I do recommend treatings
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
add a comment |
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);
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);
edited Jan 24 '16 at 18:51
answered Jan 22 '16 at 20:54
SirPython
11.9k32890
11.9k32890
2
I do recommend treatings
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
add a comment |
2
I do recommend treatings
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
add a comment |
up vote
7
down vote
You're using
printf
to print a singlechar
:
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 singlechar
tostdout
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, usefputs(aux, stdout)
instead.)
puts
andfputs
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
orfputs
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.
If usingputs()
, 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 ofstrlen()
anyway.
– David Foerster
Jan 23 '16 at 0:31
add a comment |
up vote
7
down vote
You're using
printf
to print a singlechar
:
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 singlechar
tostdout
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, usefputs(aux, stdout)
instead.)
puts
andfputs
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
orfputs
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.
If usingputs()
, 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 ofstrlen()
anyway.
– David Foerster
Jan 23 '16 at 0:31
add a comment |
up vote
7
down vote
up vote
7
down vote
You're using
printf
to print a singlechar
:
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 singlechar
tostdout
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, usefputs(aux, stdout)
instead.)
puts
andfputs
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
orfputs
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.
You're using
printf
to print a singlechar
:
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 singlechar
tostdout
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, usefputs(aux, stdout)
instead.)
puts
andfputs
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
orfputs
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.
edited Jan 29 '16 at 10:37
answered Jan 23 '16 at 0:18
David Foerster
1,600317
1,600317
If usingputs()
, 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 ofstrlen()
anyway.
– David Foerster
Jan 23 '16 at 0:31
add a comment |
If usingputs()
, 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 ofstrlen()
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
add a comment |
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)
.
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
add a comment |
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)
.
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
add a comment |
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)
.
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)
.
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
add a comment |
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
add a comment |
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.
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
add a comment |
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.
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
add a comment |
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.
You need to ensure that your aux string is null terminated. Add the line aux[strlen(s)] = '';
right after the first for loop.
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
add a comment |
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
add a comment |
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.
add a comment |
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.
add a comment |
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.
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.
edited Jan 24 '16 at 0:06
Jamal♦
30.2k11115226
30.2k11115226
answered Jan 23 '16 at 23:59
Spoofy McGee
491
491
add a comment |
add a comment |
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
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
add a comment |
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
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
add a comment |
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
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
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
add a comment |
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
add a comment |
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.
add a comment |
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.
add a comment |
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.
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.
edited 1 hour ago
albert
1371
1371
answered Jan 23 '16 at 9:59
fhtuft
1414
1414
add a comment |
add a comment |
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?
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 usingfgets( 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 asize_t
value, not aint
value. To avoid theimplicit conversions, use
size_t`– user3629249
Jan 25 '16 at 2:06