-
help with function
Hello guys,
I would like to ask you if anybody knows how to fix or upgrade my function "set". My teacher wrote me that I have to allocate the function inside not in main. And second thing is how to change that the pointers to pointers in function would be simplified.
Thanks for help
Code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#define BUFFER_SIZE 128
int set(char *in, char **out);
int main(int argc, char **argv)
{
char inStr[BUFFER_SIZE] = { 0 };
char *outStr = NULL;
int changed = 0;
printf("Exercise 10.1\n");
printf("Enter string:");
if (!fgets(inStr, BUFFER_SIZE, stdin)) { //enter string
printf("Error load string...\n");
exit(EXIT_FAILURE);
}
int lenght = strlen(inStr); //include '\n' char as last
inStr[lenght - 1] = '\0'; //remove last char '\n' from fgets function
outStr = (char *) malloc(lenght); //alloc new string of lenght
if (!outStr) {
printf("Error in alloc memory...\n");
exit(EXIT_FAILURE);
}
changed = set(inStr, &outStr); //call function
printf("Original string: %s\n", inStr);
printf("Changed string: %s and changed letters: %d\n", outStr, changed);
free(outStr); //free alloc memory
getchar();
return 0;
}
int set(char *in, char **out)
{
char *tmpString = *out; //temp variable, out cannot be used like out++
int changed = 0; //changed letters
while (*in) {
if (*in >= 65 && *in <= 90) {
*tmpString = *in + 32;
changed++;
} else if (*in >= 97 && *in <= 122) {
*tmpString = *in - 32;
changed++;
} else {
*tmpString = *in;
}
in++;
tmpString++;
}
*tmpString = '\0'; //add ending char '\0'
return changed;
}
-
First things first: next time, please post your code as plain text. That way the forum's software will be able to properly apply syntax highlighting and line numbering to your code.
Issues I see:
You don't use argc or argv, so you can declare main as int main(void) for simplicity and clarity.
Code:
int lenght = strlen(inStr); //include '\n' char as last
inStr[lenght - 1] = '\0'; //remove last char '\n' from fgets function
1. It's spelled length. Minor issue, but liable to cause annoying problems if you spell it correctly later :).
2. This does not necessarily remove the '\n' from the end of the string. It removes the last character no matter what it is. Most of the time it will be a '\n', but it may be something else (e.g. the last letter of the user's input because the input was terminated with EOF instead of '\n')
Instead, try using the strchr method found here: FAQ > Get a line of text from the user/keyboard (C) - Cprogramming.com.
Code:
if(!fgets(inStr, BUFFER_SIZE, stdin)) { //enter string
It is preferable to use sizeof(inStr) in the fgets call, instead of BUFFER_SIZE. That way, if you ever declare inStr to be a different size (e.g. FOO_SIZE), your fgets call will still be correct, you wont need to change it. Not a serious issue in a smaller program, but a good habit to get into so it's automatic when you write larger, more complex programs.
[/code]outStr = (char*)malloc(lenght); //alloc new string of lenght[/code]
Don't cast the return value of malloc, see here: FAQ > Casting malloc - Cprogramming.com. Also, since there might not be a newline, you need to make sure you always allocate enough space for the whole new string, plus the
set() is a horrible funciton name. I have no idea what it does. What is it setting? Looks more like it's swapping certain characters. How about you call it swapXandY, where you pick the appropriate words to fill in X and Y so the function name is accurate and descriptive. Never sacrifice clarity of code to save a few keystrokes. It saves you 30 seconds now, and can cost you hours later, if you have to return to your code. Again, not a serious issue in a smaller program, but form good habits early on.
Don't use magic numbers. I can guess what 65, 90, 97, 122 and 32 are for, but it's really not clear. Also, while ASCII or compatible character sets are very, very common, they are not universal. There do still exist character sets (like EBCDIC) where upper case letters are not between 65 and 90, and they are not consecutive, and they are not 32 chars away from their lowercase counterparts. Instead, check for upper/lower case with the isupper() and islower() functions, and convert them with toupper() and tolower(). You will need to #include <ctype.h> to use those function. This gives you a few benefits:
1. It is more portable: it will work no matter what character set you end up using.
2. You don't have to worry about remembering what numbers are what.
3. It uses pre-built and pre-tested functions, so you don't need to worry about making a mistake, the logic is already handled for you -- no need to reinvent the wheel.
4. It makes your code more readable. Those function names describe clearly what they do, so your code will be much easier to read:
Code:
if (isupper(*in)) {
*tmpString = tolower(*in);
} else if (islower(*in)) {
*tmpString = toupper(*in);
} else {
*tmpString = *in;
}
To malloc inside the function, and make sure outStr is correct in main, simply assign the result of malloc to *out in your swapXandY function
Code:
*out = malloc(strlen(in) + 1);
Make sure you check the return value of malloc, to ensure you actually have memory before you attempt to write to it.