Totally unexpected output when trying to compute the average using arrays

Question

I am trying to compute the average after reading in the data from a text file of int type.The program compiles fine. clang -std=gnu11 -Weverything -g3 -pedantic -g3 -O3 -lm average_weight_of_elephant_seals.c -o average_weight_of_elephant_seals

Suppose I want to compute the average weight of 2000 seals,the expected output is 6838.848152 but I get 1710.566467.I have no idea how to make sense of GDB yet.

Could someone please point out where have I have gone wrong?

/* The following program demonstrates the usage of fscan to read in a set of integer data into a file and then computes the sum followed by the average.
 * The computation shall be encapsulated in a function and then be called in the main routine
 */
#include <stdio.h>
#define MAXSIZE 5000 /* Macro definition to pre-define the size of the array */

double average_weight(int count, int weights_array[]);

int main(void)
{
    int number_of_seals;
    int weights_array[MAXSIZE];

    printf("Enter the number of seals: \n");
    scanf("%i", &number_of_seals);

    printf("Their average weight is %lf\n", average_weight(number_of_seals, &weights_array[number_of_seals]));

    return 0;
}

double average_weight(int count, int weights_array[])
{
    /* Variable declaration and initialization
     * Note the use of the FILE data type */
    int weight;
    int sum = 0;
    FILE *elephant_seal_data = fopen("elephant_seal_data.txt", "r");

    if (elephant_seal_data == NULL)
    {
        return -1;
    }

    /* FEOF function to determine if EOF has been reached or not */
    while (!feof(elephant_seal_data))
    {
        fscanf(elephant_seal_data, "%i", &weight);
        weights_array[count++] = weight;
        sum += weight;
        count++;
    }

    double average_weight = (double)sum / (double)count;
    fclose(elephant_seal_data);
    return average_weight;
}

Answer 1

printf("Their average weight is %lf\n", average_weight(number_of_seals, &weights_array[number_of_seals]));

The code passes a pointer to a position into the array for no apparent reason, and does not check if number_of_seals * 2 is less than MAXSIZE so may overflow the array. But the array isn't needed for this calculation anyway.

    weights_array[count++] = weight;
    sum += weight;
    count++;

The code is writing to the array not reading it. The array is not needed for this calculation.

The code increments count twice, so the average will be out by a factor of two, and alternate locations in the array will have undefined values in them.

Answer 2

There are 2 stupid mistakes in your code, a nastier one, and a risk.

First the stupid ones:

You pass count to the function and increment that value twice per each value in the file. If the initialy given value was correct, you end with a count 3 times too big. You should not pass count to the function but compute it there.

You use a wrong syntax to pass an array: you are expected to pass a pointer to its first element.

Now the nasty one: while Why is “while ( !feof (file) )” always wrong? is indeed a FAQ, is is still a common thing in beginners code...

feof only returns true after a read operation returned an error. Let us examine what happens for the last value. It is read and correctly processed once. feof still returns false (no error so far) so your code re-enters the loop. scanf reaches the end of file and returns 0 (what your code ignores) but does not change the values => the last value will be processed twice. Never ever use while (!feof(...

And finally the risk.

You are summing value into an integer. Even if the average will easily fit there, if you had larger value and a very high number of them, you could get an integer overflow. The recommended way it to sum into a larger type (double?) and if possible use a guess to limit the cumulative error: average(qty-guess) + guess is indeed average(quantity), but the computed sum can be much lower, limiting the cumulative error when using floating point values or preventing overflow when using integer ones. From the number of seals and the expected average there should be no problem here so a guess is useless, but remember that for a different use case...

Last but not least, main is expected to be declared as int main() if you do not care for additional parameters but never int main(void)

Code could become:

/* The following program demonstrates the usage of fscan to read in a set of integer data into a file and then computes the sum followed by the average.
 * The computation shall be encapsulated in a function and then be called in the main routine
 */
#include <stdio.h>
#define MAXSIZE 5000 /* Macro definition to pre-define the size of the array */

double average_weight(int* count, int weights_array[]);

int main()
{
    int number_of_seals;
    int weights_array[MAXSIZE];

    double weight = average_weight(&number_of_seals, weights_array);
    printf("Their number is %d and their average weight is %lf\n", number_of_seals, weight);

    return 0;
}

double average_weight(int* count, int weights_array[])
{
    /* Variable declaration and initialization
     * Note the use of the FILE data type */
    int weight;
    int sum = 0;
    FILE* elephant_seal_data = fopen("elephant_seal_data.txt", "r");

    if (elephant_seal_data == NULL)
    {
        return -1;
    }

    *count = 0;

    /* FEOF function to determine if EOF has been reached or not */
    for(int i=0; i<MAXSIZE; i++) // never process more than the array size
    {
        if (1 != fscanf(elephant_seal_data, "%i", &weight)) {
            break;    // immediately stop at end of file
        }
        weights_array[(* count)++] = weight;
        sum += weight;
    }

    double average_weight = (double)sum / (double)*count;
    fclose(elephant_seal_data);
    return average_weight;
}

---

I have kept your general program structure unchanged, but IMHO, you are expected to first read the data into an array, and then pass that populated array along with its count to an average function. Just split your current function into 2 steps.

Answer 3

You have sent the number of counts to use in the array which is great, since the function does not know the length of the weights_array. But you are not using it properly.

I'd suggest you to:

• Use count to limit the number of loops based on how many data you want.
• Do not change/reassign the value of count. Since this number is crucial to calculate the average. Create some other variable to do the task.

So here is how I slightly modified your code to bring those changes. I assumed the format of elephant_seal_data.txt as space separated integer values.

#include <stdio.h>
#define MAXSIZE 5000 /* Macro definition to pre-define the size of the array */

double average_weight(int count, int weights_array[]);

int main(void)
{
    int number_of_seals;
    int weights_array[MAXSIZE];

    printf("Enter the number of seals: \n");
    scanf("%i", &number_of_seals);

    printf("Their average weight is %lf\n", average_weight(number_of_seals, &weights_array[number_of_seals]));

    return 0;
}

double average_weight(int count, int weights_array[])
{
    /* Variable declaration and initialization
     * Note the use of the FILE data type */
    int weight;
    int sum = 0;
    int i = 0;
    FILE *elephant_seal_data = fopen("elephant_seal_data.txt", "r");

    if (elephant_seal_data == NULL)
    {
        return -1;
    }

    /* FEOF function to determine if EOF has been reached or not */
    while (i<count)
    {
        fscanf(elephant_seal_data, "%d", &weight);
        weights_array[i++] = weight;
        if (feof(elephant_seal_data)) break;
        sum += weight;
    }




    double average_weight = (double)sum / (double)count;
    fclose(elephant_seal_data);
    return average_weight;
}

Edit: I have used the elephant_seals_data.txt to simulate these in Google Colab for you. Try running the first cell there.

Google Colab Link