How to properly make a counting algorithm to count from file?

Question

This is a program to find the largest even number and its times of occurring from an input file and output it to an output file. I'm having a problem with the output, there seems to be an extra iteration that messes things up.

int main(int argc, char const *argv[])
{
    int n, num, i, even, count;

    FILE * fptr;
    FILE * fptro;

    fptr =fopen("maximpar.in", "r");
    fptro=fopen("maximpar.out", "w");

   /*scanning the first line from the file to get n for for()*/

    fscanf(fptr, "%d", &n); 

    count = 0;
    even = INT_MIN;
    for(i = 0; i < n; i++)
{
    fscanf(fptr, "%d", &num);

    if( (num % 2 == 0 && num > even) || (even == num) ) 

    /*checking  for largest even number, 
    not sure about the ..||(even == num) part of the condition*/

    {
        even = num;
        count++;
    }

}

    fprintf(fptro, "%d %d", even, count);


    fclose(fptr);
    fclose(fptro);

    return 0;
}

Input file

 6
 9 6 9 8 9 8

Output file

8 3

Why isn't the output file like this? I don't understand

8 2

score 1 · Answer 1 · answered Dec 24 '18 at 19:46

1

enclose the condition

   if(  ( ...&&...) ||(....) )

answered Dec 24 '18 at 19:46

Himanshu

3,830
2
10
29

While it may look neater like this, or possibly be crucial in some cases this didn't do anything for my problem – Rami Raghfan Dec 24 '18 at 19:52
straight away ill start with 6 (6%2==0 && 6>-1) p=6,cnt=1 next 8 (8%2==0 && 8>6) p=8,cnt=2 next 8 again ..||(8==8) cnt=3 unlike your case 6%2 && 6>-1 || 6==-1 cnt1,p=6 , 8%2 && 8>6 || 8==6 cnt=2,p=8 finally, 8%2 && 8>8 || 8==8 cnt will remain 2 – Himanshu Dec 24 '18 at 20:05

Craig Estey · Accepted Answer · 2018-12-24T20:51:12.457

You need to reset your count whenever you get a new larger number.

I didn't test this, but it should work:

cate = 0;
par = INT_MIN;

for (i = 0; i < n; i++) {
    fscanf(fptr, "%d", &b);

    // skip odd numbers
    if ((b % 2) != 0)
        continue;

    // get new larger number
    if (b > par) {
        par = b;
        cate = 1;
        continue;
    }

    // increment count on existing largest number
    if (b == par)
        ++cate;
}

UPDATE:

I dont understand why skip iterations explicitly instead of only picking out the iterations that matter? Is there some sort of advantage?

Yes, it's better style. It allows simple single level indented if statements that can have their own comments.

It avoids a messy compound if or a triple level if/else ladder.

IMO, it's a common misconception [particularly among beginning C programmers] that a complex if will execute faster [or is somehow "better"] than several simple ones.

The first if could be thought of a "skip this iteration" test. Here, there's only one. But, for more complex code, there might be several.

The multiple condtion escapes could be handled in a single if with if (c1 || c2 || c2 || ... || c10) continue; but that gets messy fast.

Herein, for properly indented if/else ladder logic, we'd need:

if (cond1)
    do_stuff1;
else
    if (cond2)
        do_stuff2;
    else
        if (cond3)
            do_stuff3;

If we're not in a loop, here's a "trick" to avoid if/else ladder logic, by using do { ... } while (0);:

do {
    if (cond1) {
        do_stuff1;
        break;
    }

    if (cond2) {
        do_stuff2;
        break;
    }

    if (cond3) {
        do_stuff3;
        break;
    }
} while (0);

I dont understand why skip iterations explicitly instead of only picking out the iterations that matter? Is there some sort of advantage? — Rami Raghfan, Dec 24 '18 at 20:19
I see, our professors do insist on ''Coding Style'' in homeworks. This is helpful. but why did my output show 3? and which iteration did it have an extra count++;? — Rami Raghfan, Dec 24 '18 at 21:14
may i ask again why skip odds instead of directly getting them? — Rami Raghfan, Dec 24 '18 at 21:24

user9201794 · Answer 3 · 2018-12-24T20:32:53.843

0

The answer is because count was incremented from 0 to 1 when b = 6. 2 iterations later, b = 8 and now count = 2, and 2 iterations after that, b = 8 and count = 3.

I also recommend you nest your if statement in parentheses for readability. Commenting would help too :) I'm a stats guy, and I have no idea what you are doing based on your variables' names.

You need to reset your counter inside the if block if b > par.

Like:

if( num % 2 == 0 && num >= even) {

if (num > even){ 

    even = num; 
    count = 1; 

} else { 

    count++; 

}

}

Thanks.

JK

edited Dec 24 '18 at 20:32

answered Dec 24 '18 at 19:56

user9201794

11
2

But theoretically my if() was executed only if it found a large number, perhaps everything after my || operator is messing something up? – Rami Raghfan Dec 24 '18 at 20:21
You need to reset your counter inside the if block if b > par. if( num % 2 == 0 && num >= even) { if (num > even){ even = b; count = 1; } else { count++; } – user9201794 Dec 24 '18 at 20:25

How to properly make a counting algorithm to count from file?

Input file

Output file

3 Answers3

Linked