Quick question, What have I done wrong here. The purpose of this code is to get the input into a string, the input being "12 34", with a space in between the "12" and "32" and to convert and print the two separate numbers from an integer variable known as number. Why doesn't the second call to the function copyTemp, not produce the value 34?. I have an index_counter variable which keeps track of the string index and its meant to skip the 'space' 开发者_开发百科character?? what have i done wrong?
thanks.
#include <stdio.h>
#include <string.h>
int index_counter = 0;
int number;
void copyTemp(char *expr,char *temp);
int main(){
char exprstn[80]; //as global?
char tempstr[80];
gets(exprstn);
copyTemp(exprstn,tempstr);
printf("Expression: %s\n",exprstn);
printf("Temporary: %s\n",tempstr);
printf("number is: %d\n",number);
copyTemp(exprstn,tempstr); //second call produces same output shouldnt it now produce 34 in the variable number?
printf("Expression: %s\n",exprstn);
printf("Temporary: %s\n",tempstr);
printf("number is: %d\n",number);
return 0;
}
void copyTemp(char *expr,char *temp){
int i;
for(i = index_counter; expr[i] != '\0'; i++){
if (expr[i] == '0'){
temp[i] = expr[i];
}
if (expr[i] == '1'){
temp[i] = expr[i];
}
if (expr[i] == '2'){
temp[i] = expr[i];
}
if (expr[i] == '3'){
temp[i] = expr[i];
}
if (expr[i] == '4'){
temp[i] = expr[i];
}
if (expr[i] == '5'){
temp[i] = expr[i];
}
if (expr[i] == '6'){
temp[i] = expr[i];
}
if (expr[i] == '7'){
temp[i] = expr[i];
}
if (expr[i] == '8'){
temp[i] = expr[i];
}
if (expr[i] == '9'){
temp[i] = expr[i];
}
if (expr[i] == ' '){
temp[i] = '\0';
sscanf(temp,"%d",&number);
index_counter = i+1; //skips?
}
}
// is this included here? temp[i] = '\0';
}
There are a few problems in your program:
- You are using the same index into
expr
andtemp
arrays. This works for the first time since both will be0
to start with but when you want to process the2nd
number, you need to reset the index into thetemp
array back to0
. Clearly this cannot be done using a single index. You'll have to use two indices,i
andj
. - By the time you complete the
processing of the 2nd number (
34
in"12 34"
) you'll reach the end of the string and hence thesscanf
never gets run on the second occasion ( in general for the last occasion). So after the for loop you need another sscanf to extract the last number. Also you should return from the function once you've extracted the number from the string and incremented i. - You should avoid using
gets
() and usefgets
() instead because of security reasons. - You can combine the multiple test for the digits into a single test as shown:
Something like this.
void copyTemp(char *expr,char *temp){
int i;
int j = 0;
for(i = index_counter; expr[i] != '\0'; i++){
if (expr[i] >= '0' && expr[i]<='9'){
temp[j++] = expr[i]; // copy the digit into temp..increment j.
}
else if (expr[i] == ' '){ // space found..time to extract number.
temp[j] = '\0'; // terminate the temp.
sscanf(temp,"%d",&number); // extract.
index_counter = i+1; // skip the space.
return; // done converting...return..must not continue.
}
}
// have reached the end of the input string..and still need to extract a
// the last number from temp string.
temp[j] = '\0';
sscanf(temp,"%d",&number);
}
After these changes it works as expected:
$ gcc b.c 2> /dev/null && ./a.out
12 34
Expression: 12 34
Temporary: 12
number is: 12
Expression: 12 34
Temporary: 34
number is: 34
Your approach is very fragile...if a user gives multiple spaces between the input numbers..your program will fail.
The main problem is that copyTemp
writes to temp[i]
, but each call to copyTemp
initializes i
to index_counter
, not to 0. This means that each call to copyTemp
appends to the existing temp
buffer instead of overwriting the old contents, and sscanf
thus always re-reads the same string. You need to use separate indices to keep track of where to read from the input buffer and where to write to the output buffer.
Additional problems:
* Never** use ggets
. Ever. Use fgets
instead.
* You duplicate a lot of code in copyTemp
. You instead could do:
if (expr[i] == '0' || expr[i] == '1' || ...)
or better:
if (isdigit(expr[i]))
copyTemp
should take some precautions to not overflow its destination buffer. (Note thatcopyTemp
shouldn't even need to take a destination buffer as an argument.)You should avoid using global variables. It'd be better for
copyTemp
to take an argument specifying where to start reading from the input string and if it returned the index where it left off.
精彩评论