# I'm sure I can solve this with a function I haven't heard about yet.

This is a discussion on I'm sure I can solve this with a function I haven't heard about yet. within the C Programming forums, part of the General Programming Boards category; Hi, So, I have a file parser that, at one point, takes input of the form of "h(124) c(566) f(178)" ...

1. ## I'm sure I can solve this with a function I haven't heard about yet.

Hi,

So, I have a file parser that, at one point, takes input of the form of "h(124) c(566) f(178)" and reads it into variable array current [7]. It will only accept l(), h(), a(), v(), c(), or f(), and stores each in a corresponding place in current. Now, I managed to make this work, but I think it could be condensed to a little less than one sixth of the code. Paraphrased:

Code:
IGNOREWHITESPACE(props)
if (buf == '&') {
IGNOREWHITESPACE(props)
if (buf == 'l') {
while (buf != '(')
buf = fgetc(props);
for (int i = 0; i <= 2; i++) {
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
}
current [0] = atoi(num);
while (buf == ')') {
buf = fgetc(props);
}
strcpy(num, "!!!");
IGNOREWHITESPACE(props)
}
if (buf == 'h') {
while (buf != '(')
buf = fgetc(props);
for (int i = 0; i <= 2; i++) {
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
}
current [1] = atoi(num);
while (buf == ')') {
buf = fgetc(props);
}
strcpy(num, "!!!");
IGNOREWHITESPACE(props)
}
if (buf == 'a') {
while (buf != '(')
buf = fgetc(props);
for (int i = 0; i <= 2; i++) {
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
}
current [2] = atoi(num);
while (buf == ')') {
buf = fgetc(props);
}
strcpy(num, "!!!");
IGNOREWHITESPACE(props)
}
if (buf == 'v') {
while (buf != '(')
buf = fgetc(props);
for (int i = 0; i <= 2; i++) {
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
}
current [3] = atoi(num);
while (buf == ')') {
buf = fgetc(props);
}
strcpy(num, "!!!");
IGNOREWHITESPACE(props)
}
if (buf == 'c') {
while (buf != '(')
buf = fgetc(props);
for (int i = 0; i <= 2; i++) {
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
}
current [4] = atoi(num);
while (buf == ')') {
buf = fgetc(props);
}
strcpy(num, "!!!");
IGNOREWHITESPACE(props)
}
if (buf == 'f') {
while (buf != '(')
buf = fgetc(props);
for (int i = 0; i <= 2; i++) {
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
}
current [5] = atoi(num);
while (buf == ')') {
buf = fgetc(props);
}
strcpy(num, "!!!");
IGNOREWHITESPACE(props)
}
else
return 2;
}
else
return 1;
return 0;
}
Yeah. Literally the only thing that changes is the index of current that it writes it to. Is there any way to solve this so it's tinier, with the exact same result? Maybe putting it in a for cycle would be the first step?

2. Why don't you write a function that contains the repetitive code and takes as argument the different index and pointers to whatever other variables are required for I/O?

Then you can just call it as my_func(index,.....);. Also change your ifs to a switch, remember that characters are essentially represented as integers.

3. Code:
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
since you are using fgetc, if a user types '1', you get the character '1', not the numeric value 1. so I think you would want to check against '1','2','3' etc. But there is an easier way to do that. try
Code:
while(!isdigit(num[1])) {
num[i] = fgetc(props);
}
you have to #include <ctype.h> to get isdigit

4. Originally Posted by claudiu
Why don't you write a function that contains the repetitive code and takes as argument the different index and pointers to whatever other variables are required for I/O?

Then you can just call it as my_func(index,.....);. Also change your ifs to a switch, remember that characters are essentially represented as integers.
I don't want to make my variables global, though, and I'm not very good with pointers to arrays. I guess I could make it return the value to put into current, though! Thanks, I'll try that.

Originally Posted by dmh2000
Code:
while (num [i] != 0 || num [i] != 1 || num [i] != 2 || num [i] != 3 || num [i] != 4 || num [i] != 5 || num [i] != 6 || num [i] != 7 || num [i] != 8 || num [i] != 9)
num [i] = fgetc(props);
since you are using fgetc, if a user types '1', you get the character '1', not the numeric value 1. so I think you would want to check against '1','2','3' etc. But there is an easier way to do that. try
Code:
while(!isdigit(num[1])) {
num[i] = fgetc(props);
}
you have to #include <ctype.h> to get isdigit
Hmm, you're right about that. I meant to put those in single quotes. 2 AM is definitely not the most productive time to code. Thanks for telling me about isdigit, though; much cleaner!

5. A dislike for global variables is good, but it looks like props, buf, num and current appear to be globals anyway. Also, avoiding "difficult" stuff like pointers to arrays (which you don't even really need) is not the way to improve your skills. I say you don't need them because, if you want to change the values in an array, you only need to pass a pointer to it's first element. Think of the strcpy function, it doesn't take a complicated char (*dest)[] parameter, just a char *dest.

This basically looks like you are scanning a file for specifically formatted data. How about using fscanf (note, this is untested, it's purely conceptual):
Code:
char type[2];  // one char plus a null terminator
int value, index;
if (fscanf(props, "%1[lhavcf](%d)", type, &value) == 2) {  // make sure fscanf worked
switch (type[0]) {
case 'l':
index = 0;
break;
...
}
current[index] = value;
}
The %1[lhavcf] grabs a string that is 1 character long, and consists only of the characters inside the [ ], namely 'l', 'h', 'a', 'v', 'c', 'f'. That character/string must be followed by an open parenthesis, an integer and a close parenthesis. You can limit the number of characters in the integer in a similar manner to limiting the string to one character.

Alternatively, you could make a string of valid chars for type, and use strchr with some pointer arithmetic to calculate the index.

6. Your while loop on line 9, and all the other ones like it are infinite loops.
This much of it:
Code:
num [i] != 0 || num [i] != 1
amounts to true because there do not exist any values for which it is false.
Zero? Nope zero is not equal to 1, so we get false | true ==> true.
One? Nope one is not equal to zero, so we het true | false ==> true.
Two? Nope...
Have a think about it and see if you can work out the mistake.

7. Originally Posted by iMalc
Your while loop on line 9, and all the other ones like it are infinite loops.
This much of it:
Code:
num [i] != 0 || num [i] != 1
amounts to true because there do not exist any values for which it is false.
Zero? Nope zero is not equal to 1, so we get false | true ==> true.
One? Nope one is not equal to zero, so we het true | false ==> true.
Two? Nope...
Have a think about it and see if you can work out the mistake.
Yeah, sorry, I meant to && them. :P I'm so absentminded sometimes... it's all a moot point, though, because I'm using isdigit(num [i]) now.

8. Originally Posted by anduril462
A dislike for global variables is good, but it looks like props, buf, num and current appear to be globals anyway. Also, avoiding "difficult" stuff like pointers to arrays (which you don't even really need) is not the way to improve your skills. I say you don't need them because, if you want to change the values in an array, you only need to pass a pointer to it's first element. Think of the strcpy function, it doesn't take a complicated char (*dest)[] parameter, just a char *dest.

This basically looks like you are scanning a file for specifically formatted data. How about using fscanf (note, this is untested, it's purely conceptual):
Code:
char type[2];  // one char plus a null terminator
int value, index;
if (fscanf(props, "%1[lhavcf](%d)", type, &value) == 2) {  // make sure fscanf worked
switch (type[0]) {
case 'l':
index = 0;
break;
...
}
current[index] = value;
}
The %1[lhavcf] grabs a string that is 1 character long, and consists only of the characters inside the [ ], namely 'l', 'h', 'a', 'v', 'c', 'f'. That character/string must be followed by an open parenthesis, an integer and a close parenthesis. You can limit the number of characters in the integer in a similar manner to limiting the string to one character.

Alternatively, you could make a string of valid chars for type, and use strchr with some pointer arithmetic to calculate the index.
Wooow. That's exactly the sort of thing that I'm looking for! Thanks, it's always good to learn new things. Jesus, this is pretty spot-on. That's brilliant. Thanks a lot! Oh, and it seems you know something about passing arrays into subroutines. So, I write the name of the array, and declare it with a single dereference as an array in the argument when I'm declaring the subroutine?

9. Originally Posted by mszegedy
So, I write the name of the array, and declare it with a single dereference as an array in the argument when I'm declaring the subroutine?
If I understand you correctly, yes, that is how you do it. Post an example if you want, I will tell you if you're correct.