Stop reading when reading mutliple files (without goto)

Question

I have files with a single column full of numbers and I want to sum all the files. The problem is to be able to exit as soon as invalid data is found (as soon as fprintf fails).

A first attempt using goto (and inspired by this answer) is the following

while(1) {
    sum_a = 0;
    for(i=0 ; i<N ;i++){
        if(fscanf(infiles[i], "%d\n", &a) != 1){
            goto end; // yeah, I know...
        }
        sum_a += a;
    }
    printf("%d\n", sum_a); // Should NOT be read if anything failed before
}

end:
for(i=0 ; i<N ;i++)
    fclose(infiles[i]);

I would glad to see a nicer solution (ie not using goto and while(1)!). I don't want to mess up with many flags set and unset in a dirty way.

This question is probably a good candidate for http://codereview.stackexchange.com/, instead of StackOverflow. — Leandros, May 12 '16 at 09:27

score 7 · Accepted Answer · answered May 12 '16 at 09:23

7

Why?

It's a perfectly valid use case of goto. It's the most readable solution, and does the job. Why bother changing it?

answered May 12 '16 at 09:23

Leandros

16,805
9
69
108

OK, so then no problem. I just want no raptor in my office ;) – styko May 12 '16 at 09:24

Sergey Kalinichenko · Answer 2 · 2016-05-12T12:12:09.120

Assuming that you wish to avoid this perfectly legitimate use of goto as a learning exercise, here is how you can do it: make a flag variable that indicates loop termination, set it in the inner loop, and check it in all loops:

int stop = 0;
while(!stop) {
    sum_a = 0;
    for(i=0 ; i<N ;i++){
        if(fscanf(infiles[i], "%d\n", &a) != 1){
            stop = 1;
            break;
        }
        sum_a += a;
    }
    if (!stop) {
        printf("%d\n", sum_a);
    }
}
for(i=0 ; i<N ;i++)
    fclose(infiles[i]);

Note how your code instantly became less readable, because printf needs to be protected by a conditional.

Another trick would be rewriting with a single loop and using break to stop. The readability would not suffer as much:

int i = 0;
for (;;) { // This "forever" loop is idiomatic in C
    if(fscanf(infiles[i], "%d\n", &a) != 1) {
        break;
    }
    sum_a += a;
    if (++i == N) {
        printf("%d\n", sum_a);
        i = 0;
    }
}
for(i=0 ; i<N ;i++)
    fclose(infiles[i]);

GMichael · Answer 3 · 2016-05-12T09:33:50.857

2

You can try the following:

int rc = 1;
do
{
    sum_a = 0;
    for(i=0 ; i<N ;i++){
        rc = fscanf(infiles[i], "%d\n", &a);
        if(rc == 1){
            sum_a += a;    
            printf("%d\n", sum_a);
        }        
    }
}while(rc == 1);

for(i=0 ; i<N ;i++)
    fclose(infiles[i]);

edited May 12 '16 at 09:33

answered May 12 '16 at 09:29

GMichael

2,726
1
20
30

@dasblinkenlight I modified the example – GMichael May 12 '16 at 09:34
Now your code prints too much - once per iteration of the inner loop, while OP's prints once per iteration of the outer loop. – Sergey Kalinichenko May 12 '16 at 09:35
Yes, you're right. The problem is that I do not understand the need for two loops in this case. – GMichael May 12 '16 at 09:44
OP reads `N` files until one of them runs out. At this point he stops all reading. – Sergey Kalinichenko May 12 '16 at 09:46
Thanks. Now I see it. – GMichael May 12 '16 at 09:48

joop · Answer 4 · 2016-05-12T10:00:28.713

This may look ugly (it is), but it is short and minimal:

sum_a = 0;
i =0;
while(1) {
        if(fscanf(infiles[i], "%d\n", &a) != 1) break;
        sum_a += a;
        if (++i < N) continue;
        printf("%d\n", sum_a);
        break;
        }

for(i=0 ; i<N ;i++)
    fclose(infiles[i]);

Maybe a bit less ugly:

sum_a = 0;
for(i=0 ; i<N ;i++){
    if(fscanf(infiles[i], "%d\n", &a) != 1) break;
    sum_a += a;
    }

if (i == N) printf("%d\n", sum_a);

for(i=0 ; i<N ;i++)
    fclose(infiles[i]);

Lithy · Answer 5 · 2016-05-12T10:25:37.957

When I need a big exit jump, I usually encapsulate the loops into a function and simply use return, e.g.:

void read(FILE*[] infiles, int N)
{
    for(;;) {
        sum_a = 0;
        for(i=0 ; i<N ;i++){
            if(fscanf(infiles[i], "%d\n", &a) != 1){
                return;
            }
            sum_a += a;
        }
        printf("%d\n", sum_a);
    }
}

void cleanup(FILE*[] infiles, int N)
{
    for(i=0 ; i<N ;i++)
        fclose(infiles[i]);
}

read(infiles, N);
cleanup(infiles, N);

It usually also helps reading the code.

Stop reading when reading mutliple files (without goto)

5 Answers5