current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

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. Bear with me in this long code.So ,I wrote the code for this and I ran it and it works.Here's the code.I want to know 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;
}
share|improve this question
    
You seem to have forgotten the value: printf("The number of elements above the average is %d:"); –  Loki Astari May 2 '13 at 11:36
    
Please indent the code before posting it here. –  Lundin May 6 '13 at 6:36

3 Answers 3

up vote 1 down vote

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;
}
share|improve this answer
    
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. –  Loki Astari May 2 '13 at 11:33
up vote 1 down vote

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.

share|improve this answer
up vote 0 down vote

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.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.