Displaying the number of elements larger than the average of an array

Question

I have to write a program in C that reads an array with n elements and then displays how many elements are bigger than the average of the elements of the vector.

Is there any other way to do this in a shorter or simpler way?

#include <stdio.h>
#include <stdlib.h>
#define SIZE 100
void readArray(int [] , int );
double findAverage(int [] , int);
int countAboveAverage(int [] , int , double);
int main(int argc, char* argv []) {
//declare an array of size 100
int arr[SIZE];
int n;
printf("Enter a value for n: ");
scanf("%d", &n);
if(n==0) {
fprintf(stderr, "Your size of n is too big. Run the program again.\n");
exit(1);
}
//this function will do the reading
readArray(arr, n);
//this is a function to find the average
double average = findAverage(arr, n);
//this is a function that will count the number above average
int count = countAboveAverage(arr, n, average);
printf("The number of elements above the average is %d:");
return EXIT_SUCCESS;
}
void readArray(int arr[], int n) {
int i;
for(i=0; i<n; i++) {
printf("Please enter #%d: ", i+1);
scanf("%d", arr + i); /* or &arr[i] */
}
}
double findAverage(int arr[], int n) {
int sum = 0;
int i;
for(i=0; i<n; i++)
sum+=arr[i];
return (double)sum / n;
}
int countAboveAverage(int arr[], int n, double average) {
int count=0;
int i;
for(i=0; i<n; i++)
count = arr[i] > average ? count+1: count;
return count;
}

• Loki Astari: You seem to have forgotten the value: printf("The number of elements above the average is %d:");
• Lundin: Please indent the code before posting it here.

Answer 1

A few extra comments:

• define main at the bottom so that prototypes are unnecessary
• define local functions 'static'
• if you are going to use EXIT_SUCCESS then also use EXIT_FAILURE
• your comments are just noise (no useful information) and should be deleted
• add a \n at the end of the final printf (which is missing count)
• you don't check for n being greater than SIZE, only for zero (although the error printf says n is too big)

• define loop variables in the loop and always use braces, even when not necessary:

  for (int i=0; i<n; ++i) {
   // do something
  }
  

• findAverage and countAboveAverage should declare arr as const

  static double findAverage(const int arr[], int n);
  

• personally I'd avoid using unsigned types (suggested by @Sulthan) as it often leads to unexpected arithmetic results. If you do use it, make sure you enable the necessary compiler warnings.

Answer 2

The program is very simple and making it shorter is almost impossible. What I really miss is some kind of indentation but maybe this is caused by pasting the code here.

for(i=0; i<n; i++)
 count = arr[i] > average ? count+1: count;

This code is correct, but not obvious, I would simplify it to

for(i=0; i<n; i++)
 if (arr[i] > average) count++;

I am not a big fan of ignoring braces, so I would write it in the following way

for(i=0; i<n; i++) {
 if (arr[i] > average) {
 count++;
 }
}

Also, I would recommend you to look up some C/C++ code standards and use them. For most people the program will be more readable with some additional whitespaces, e.g. after keywords and arround assignments/comparisons:

for (i = 0; i < n; i++) {
 if (arr[i] > average) {
 count++;
 }
}

Also, on some places you can consider using unsigned types, e.g. for the count.

Answer 3

How about this change to countAboveAverage. For each iteration, there is an assignment whether or not "count" has changed.

int countAboveAverage(int arr[], int n, double average) {
 int count=0, i;
 for(i=0; i<n; i++) {
 if(arr[i]>average)
 count++;
 }
 return count;
}

• Loki Astari: Personally I think the original code is nicer to read. Anyway cost of assignment in such a tiny loop is unlikely to have any performance benefits thus the difference between the two versions is cosmetic.
• Brythan: @LokiAstari I find this version much easier to read. If above average, increment the count. The other code checks if it is above average and does what is essentially a no-op if it is not (it assigns one more than the count otherwise). That's much less intuitive to me (not to mention much longer to explain). It seems like it is doing something either way but actually it isn't.
• Loki Astari: @Brythan: I think that was the point I was making and as such my comment still stands. The difference here is merely cosmetic. It depends on which you like best (there is no real benefit to either). Thus not worth bringing up in code review. 30% people prefer method A, 30% people prefer method B, 40% don't care.

Answer 4

You risk an integer overflow in your findAverage function. You can avoid this by changing sum to type long. You could also change it to double, but that would increase your chance of precision loss.

double findAverage(int numbers[], int count) {
 long sum = 0;
 int i;
 for ( i = 0; i < count; i++ )
 sum+=arr[i];
 return (double)sum / count;
}

I also renamed arr and n to numbers and count respectively. In my opinion, this makes them a bit easier to understand.