2

Recently I was learning about arrays passing to functions (by passing their base address to a pointer defined as parameter in function and then using pointer arithmetic for extracting the whole array subsequently)
For practice I was asked to calculate the average marks of a class of 70 students with their marks listed in an array named "marks" and was asked to define a variable with parameter as a pointer and calculate average from there.
The data given to me was that student 1 scored 40 , student 2 scored 41, student 3 scored 42....and so on.

Here is my attempt at it:

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

float average(int *b)
{
    int sum = 0;
    for (int i = 1; i <= 70; i++)
    {
        sum = sum + *b;
        b = b + 1;
    }
    printf("the value of sum is %d\n", sum); // this value is changing every time I run the program
    return (((float)sum) / 70);
}

int main()
{
    int marks[70];
    marks[0] = 40;

    for (int i = 0; i < 68; i++)
    {
        marks[i + 1] = marks[i] + 1;
    }

    printf("the value of marks of 10th child is %d\n", marks[9]); // Just for checking if I am correct!(yes! the ans does come out to be 49!)
    printf("the value of average marks of the class is %f\n", average(&marks[0]));

    return 0;
}

to my surprise the value kept changing every time I ran it. Can anyone give a hint where am I wrong?

WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • 1
    Couple unrelated things: You're passing `&marks[0]` as an argument when u can also just directly write `marks`. Also its good practice to always zero-initialize any arrays you create (int marks[70] = {0};`). – Jakob Sachs Sep 15 '22 at 10:32
  • Thanks! Yes I remember "the name of an array represents the base address of the array" and can be replaced by `&marks[0]` – Damstridium Sep 15 '22 at 10:34
  • 3
    off by one error; you needed `i <= 68` or `i < 69` (or `i < N - 1` after variabilization). As it stands, last element of `marks` array is uninitialized so who knows what's in there, hence getting different values – Mustafa Aydın Sep 15 '22 at 10:36
  • 1
    Formatted code: it's what's for dinner. – WhozCraig Sep 15 '22 at 10:37
  • 1
    I'd suggest to *always* pass to size of the array, not only the pointer. Your `average` function has now a "magic" number (70), so that it can use only 70-wide arrays. – Bob__ Sep 15 '22 at 14:41
  • See [this excellent explanation](https://stackoverflow.com/a/1461449/2472827) of why this is the case. – Neil Sep 15 '22 at 17:55

2 Answers2

1

You're problem is related to the fact (as mentioned in my comment) that your array is uninitialized.

The memory for it has been allocated but its still random jumble data. Luckily you overwrite that data for all entries in the array, except for the last one. The last entrance is basically a random value at this point.

So thats the reason the output keeps changing, your actuall bug is a bit simpler.

In the for loop where you calculate the sum, you iterate from i = 0 to i = 67. So with the +1 offset you change all the entries from 1 to 68, so the last entrance (marks[69]) doesn't get touched.

Fixed code:

#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
float average(int *b) {
  int sum = 0;
  for (int i = 1; i <= 70; i++) {
    sum = sum + *b;
    b = b + 1;
  }
  printf("the value of sum is %d\n",
         sum); // this value is changing every time I run the program
  return (((float)sum) / 70);
}

int main() {
  int marks[70];
  marks[0] = 40;
  for (int i = 0; i < 68; i++) {
    marks[i + 1] = marks[i] + 1;
  }
  printf("the value of marks of 10th child is %d\n",
         marks[9]); // Just for checking if I am correct!(yes! the ans does come
                    // out to be 49!)
  printf("the value of average marks of the class is %f\n", average(&marks[0]));

  return 0;
}

PS: In the average function ,you use pointer arithmetic to loop over the input array, which is considered bad practice by a lot of people. Also, youre basiaclly not using the for-loop incrementor variable you create (int i). A easier and safer way to do this is :

float average(int *b) {
  int sum = 0;
  for (int i = 0; i < 69; i++) {
    sum += b[i];
  }
  printf("the value of sum is %d\n",
         sum); // this value is changing every time I run the program
  return (((float)sum) / 70);
}
Jakob Sachs
  • 669
  • 4
  • 24
  • 1
    Yes! thankyou so much ! actually our instructor told us that there was no difference between `int b[]` and `int*b`, and told us that when we send in our array to the function we dont really send in the whole array but just the base address when we type "average(marks)" and so `int b[]` and `int*b` are one and the same! and that `int b[]` was just equivalent to a pointer. since then I developed this risky habit. I will not use this anymore ! thanks! – Damstridium Sep 15 '22 at 10:51
  • Or go the other way and have the pointer be the index, `for(const int *end = b + 70; b < end; b++) sum += *b;`. – Neil Sep 15 '22 at 17:41
  • Sir in your fixed code kindly change 68 to 69 so that this doesn't confuse people!! thanks!!! – Damstridium Sep 22 '22 at 13:11
1

I was asked to calculate the average marks of a class of 70 students with their marks listed in an array named "marks"

In the posted code, we can find the following declaration

int marks[70];

The size is correct, but note that it's uninitialized, so that the values of its elements are undeterminated. The code, though, tries to assign the correct values immediately after.

marks[0] = 40; // "student 1 scored 40, student 2 scored 41, student 3 scored 42...
               //  and so on." 

for (int i = 0; i < 68; i++)
{ //            ^^^^^^ 
    marks[i + 1] = marks[i] + 1;
} //      ^^^^^ 

The last mark assigned is marks[67 + 1], which leaves marks[69] (the actual last element of the array) undeterminated.

There are many ways to achive the correct result, some may find the following appropriate.

int marks[70] = { 40 };

for (int i = 0; i + 1 < 70; ++i)
{ //         ^  ^^^^^^^^^^ 
    marks[i + 1] = marks[i] + 1;
}    

I was learning about arrays passing to functions (by passing their base address to a pointer defined as parameter in function and then using pointer arithmetic for extracting the whole array subsequently)

Using indices often yields more readable code, so I won't use "pointer arithmetic" in the following snippet.

I'd suggest to always pass the size of the array, not only the pointer.

Note how many "magic" numbers, like 70 and the wrong 68, are spread (and repeated) throughout the posted code, limiting its generality and rising the chances of errors.

I'd also extract the logic of the previous snippet into a separate function, easily modifiable and testable.

#include <stdio.h>

double average(size_t n, int *marks)
{
  long sum = 0;
  for (size_t i = 0; i < n; ++i)
  {
    sum += marks[i];
  }
  return (double)sum / n;
}

void fill_linear(size_t n, int *arr, int first_value)
{
  for (size_t i = 0; i < n; ++i)
  { 
    arr[i] = first_value++;
  }
}

int main(void)
{
  enum { n_students = 70 };

  int marks[n_students];
  fill_linear(n_students, marks, 40);

  printf("Average: %.1f\n", average(n_students, marks));  // Average: 74.5
  return 0;
}
Bob__
  • 12,361
  • 3
  • 28
  • 42