Why this C program can calculate the min but can't calculate the max?

Question

I'm learning C, and I wrote a program that reads a list of numbers and provides the sum, max, min and mean of the list. The list ends when a negative number is typed.

#include <stdio.h>
#include <stdlib.h>

int main ()
{
    int i, number, sum, divider, min, max;
    double mean;
    int addend[1000];
    char s[80];

    for (i=0; i>=0; i++) {
        fgets (s, sizeof(s), stdin);
        number = atoi(s);
        addend[i]=number;
        if (addend[i]>=0) {
            continue;
        }
        else break;
    }
    divider=i;
    i=i-1;
    sum=addend[i];
    while (i>=1) {
        sum=sum+addend[i-1];
        i=i-1;
    }
    printf("[SUM]\n%i\n", sum);
    if (addend[0]<0) {
        printf("[MIN]\n0\n\n[MAX]\n0\n\n[MEAN]\n0\n");
    }
    else {
        mean=sum/divider;
        i=divider-1;
        min=addend[i];
        while (i>=0) {
            if (addend[i-1]<min) {
                min=addend[i-1];
            }
            i=i-1;
        }
        max=addend[i];
        while (i>=0) {
            if (addend[i-1]>max) {
                max=addend[i-1];
            }
            i=i-1;
        }
        printf("[MIN]\n%i\n\n[MAX]\n%i\n\n[MEAN]\n%f\n", min, max, mean);
    }
    return 0;
}

Everything works fine, except the max (if i type "3, 6, 8, 9, -1" the max is 1075314688). I already found a solution (if I write max=addend[divider-1] on line 42 it works fine), but I'm trying to understand why the code I originally wrote doesn't work. I tried searching for the answer online but I didn't find anything.

In your question, you wrote: "but I'm trying to understand why the code I originally wrote doesn't work" -- Have you tried running your original code line-by-line in a debugger while monitoring the values of all variables, in order to determine in which line your program stops behaving as intended? If you did not try this, then you may want to read this: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/q/25385173/12149471) You may also want to read this: [How to debug small programs?](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/) — Andreas Wenzel, Aug 24 '22 at 11:31
`for (i=0; i>=0; i++)` Where did you get that weird condition? If you don't enter a sentinel value, your loop will happily run until you access `addend` beyond its limits and even until `i` overflows. That condition does not make any sense. If you want unconditioned loop, use `while` or `do while`. Or just use `true` as condition. But what you really should do is check that you do not cause buffer overflow on `addend`. — Gerhardh, Aug 24 '22 at 11:48
This is completely tangential to your main problem. You have: `if (addend[i]>=0) { continue; } else break;` —— It would be simpler to use `if (addend[i] < 0) break;`, and even if you keep the current conditional, consistency suggests you should either use braces twice (once around `continue` and once around `break` or not at all. Consistency is important in programming; it makes it easier to read the code. — Jonathan Leffler, Aug 24 '22 at 13:17

Persixty · Answer 1 · 2022-08-24T12:48:29.657

You don't reset i when you're recalculating the max. You need to insert i=divider-1; before max=addend[i]; like you did for the minimum:

i=divider-1;
min=addend[i];
while (i>=0) {
    if (addend[i-1]<min) {
        min=addend[i-1];
    }
    i=i-1;
}
i=divider-1; //Unless you do this i==-1 at this point!
max=addend[i];
while (i>=0) {
    if (addend[i-1]>max) {
        max=addend[i-1];
    }
    i=i-1;
}

PS: You should learn about for(;;) loops. This code is asking to be converted in to standard loops. I'm also not sure why you have (i>=0) and then addend[i-1] when i==0 doesn't that access addend[-1]?

Vlad from Moscow · Accepted Answer · 2022-08-24T12:57:46.057

In general this code snippet

i=i-1;
sum=addend[i];

can invoke undefined behavior if the first entered number will be negative. In this case i will be equal to 0 and the expression i - 1 will be equal to -1. That means that the expression addend[i] accesses memory beyond the array.

This while loop invokes undefined behavior

    i=divider-1;
    min=addend[i];
    while (i>=0) {
        if (addend[i-1]<min) {
            min=addend[i-1];
        }
        i=i-1;
    }

because when i is equal to 0 the expression addend[i-1] accesses memory beyond the array addend.

And moreover after above loop is equal to -1 so this statement

max=addend[i];

again invokes undefined behavior and the following loop

    while (i>=0) {
        if (addend[i-1]>max) {
            max=addend[i-1];
        }
        i=i-1;
    }

will not be executed.

Also pay attention to that in this statement in the right-side of the assignment

mean=sum/divider;

there is used the integer arithmetic.

The program can look the following way.

#include <stdio.h>
#include <stdlib.h>

int main( void ) 
{
    enum { N = 1000 };
    int addend[N];
    char s[80];

    size_t n = 0;

    while ( n < N && fgets (s, sizeof( s ), stdin ) != NULL )
    {
        int number = atoi( s );
        if ( !( number < 0 ) )
        {
            addend[n++] = number;
        }
        else
        {
            break;
        }
    }

    if ( n == 0 )
    {
        printf("[MIN]\n0\n\n[MAX]\n0\n\n[MEAN]\n0\n");
    }
    else
    {
        long long int sum = 0;
        for ( size_t i = 0; i < n; i++ )
        {
            sum += addend[i];
        }

        printf( "[SUM]\n%lld\n", sum );

        int min = addend[0];
        int max = addend[0];

        for ( size_t i = 1; i < n; i++ )
        {
            if ( max < addend[i] )
            {
                max = addend[i];
            }
            else if ( addend[i] < min )
            {
                min = addend[i];
            }
        }

        double mean = ( double )sum / n;

        printf( "[MIN]\n%i\n\n[MAX]\n%i\n\n[MEAN]\n%f\n", min, max, mean);
    }

}

Why this C program can calculate the min but can't calculate the max?

2 Answers2