Calculate sum, mean, sum of squares, and standard deviation of array elements

Question

Also, I've commented out a function I was trying to do to count the elements of an array; I realized that is not possible due to the fact that I won't be be able to return a NULL on an array.

Although this part of a beginner to C exercise, I'd like to know if there is anything I can do to improve my coding.

#include <stdio.h>
#include <math.h>
#define MAX_ITEM 1000
double x[MAX_ITEM];
int arr_count(double x[]);
double sum(double x[], int arr_count);
double mean(double sum, int arr_count);
double sum_sqr(double x[], int arr_count);
double st_dev(double sum_sqr, double mean, int arr_count);
/*
int arr_count(double x[]) {
 int i = 0;
 int count = 0;
 for (i = 0; i < MAX_ITEM; i++) {
 if (x[i] != NULL){
 count++;
 }
 }
 return count;
}
*/
double sum(double x[], int arr_count){
 int i = 0;
 double my_sum = 0;
 for (i = 0; i < arr_count; i++){
 my_sum += x[i];
 }
 return my_sum;
}
double mean(double sum, int arr_count) {
 return sum / arr_count;
}
double sum_sqr(double x[], int arr_count){
 int i = 0;
 double my_sum = 0;
 for (i = 0; i < arr_count; i++){
 my_sum += x[i] * x[i];
 }
 return my_sum;
}
double st_dev(double sum_sqr, double mean, int arr_count) {
 return sqrt(sum_sqr/ arr_count - mean * mean);
}
int main(void){
 int i;
 int num;
 int my_arr_count = 0;
 double my_sum = 0;
 double my_mean = 0;
 double my_st_dev = 0;
 printf("Enter the number of elements you want in the array between 0 and 1,000\n");
 scanf("%d", &num);
 //generate elements
 for (i = 0; i < num; i++) {
 x[i] = rand(time(NULL));
 }
 // print the generated elements
 printf("The elements are:\n");
 for (i = 0; i < num; i++){
 printf("[%d] %lf\n", i+ 1, x[i]);
 }
 // my_arr_count = arr_count(x);
 my_arr_count = num;
 my_sum = sum(x, my_arr_count);
 my_mean = mean(my_sum, my_arr_count);
 my_st_dev = st_dev(sum_sqr(x,my_arr_count), my_mean, my_arr_count);
 printf("There are %d elements in the random array", my_arr_count);
 printf("the Sum is %lf", my_sum);
 printf("The Mean is %lf", my_mean);
 printf("The Standard Deviation is %lf", my_st_dev);
}

Answer 1

see where you have done

 //generate elements
 for (i = 0; i < num; i++) {
 x[i] = rand(time(NULL));
 }

Thats a good indication you need a function. Same deal for the other place you put a comment

x shouldn't be a global. Should just be a local in main.

• dassouki: Thanks! In other words, any loop, condition statement, or data manipulation can be made into its own function. I'm assuming this has to do with code maintenance and debugging as well as easy of read/use?
• Chad: Actually anytime you have a logical process that includes multiple steps. You do not neceesarily need a function to add two numbers. But if you have a process that adds two numbers then does something then adds the result that would be a logical process that should be its own function.
• Keith Nicholas: its more to do with design, but it does lead to easier to understand code, and easier to debug. If you spend time creating functions that make it easier to express ideas in code around your problem domain, you tend to create less code, cleaner code, and also you start thinking at a higher level. As opposed to thinking about the mechanics of creating loops, branching, managing resources, adding numbers, etc to achieve the result you want.

Answer 2

Let the compiler help you

The first thing to do is to compile with a decent compiler and look at the warnings. With gcc -O -Wall -W:

dassouki.c: In function ‘main’:
dassouki.c:70: warning: implicit declaration of function ‘rand’
dassouki.c:70: warning: implicit declaration of function ‘time’
dassouki.c:90: warning: control reaches end of non-void function

You're missing two headers, stdlib.h and time.h, where the functions rand and time are declared. Once you add them you see another error:

small_fixes.c: In function ‘main’:
small_fixes.c:72: error: too many arguments to function ‘rand’

For now, to get code that compiles, remove the arguments to rand, it doesn't take any. I'll explain about rand below.

The last warning is there because main function returns an int value; you neglected to do that. If your implementation conforms to C99 (few do, and the one I compiled with doesn't), there is a special dispensation for the main function: you can omit the return statement, and it's as if you'd written return EXIT_SUCCESS. For utmost portability, return EXIT_SUCCESS to indicate success, or EXIT_FAILURE to indicate failure. On unix and Windows, EXIT_SUCCESS is 0 and EXIT_FAILURE is 1, though any nonzero value (positive and up to 255, though you should stick to small values) indicates failure. If you don't return a value from main (and aren't using a C99 compiler), the effect on most platforms is that your program returns whatever was in a particular register when main finishes executing, which is not good.

General style

Generally speaking, your code is well-organized and well-presented, congratulations. Here are a few minor improvements:

• The array x doesn't actually need to be global, you can make it local to main. If you make it global, give it a longer name — global variables should be used sparingly and should be easily noticeable and searchable.
• Don't use abbreviations in the names of global variables or functions. Call your functions square_sum or sum_of_squares, standard_deviation.
• The proper type for an array length is size_t. On some machines, you can have arrays whose size doesn't fit into an int. Note that size_t is an unsigned type, which has the advantage that you won't run into the occasional difficulties of signed arithmetic, but you need to be careful with downwards loops (for (i=n; i>=0; i--) is an infinite loop if i is unsigned).

Output

• You're missing newlines (\n) at the end of several printf calls.
• The printf conversion specifier for a double is %f (or %e or %g), not %lf. Many implementations allow %lf as a synonym for %f, but this is not universal.

• The printf conversion specified for a size_t is %z, but that only exists if your implementation conforms to C99, which is still not the norm today. In C89 (the prevalent standard today), your best bet is to cast to unsigned long, e.g.

  printf("[%lu] %lf\n", (unsigned long)i, x[i]);
  

• Why are you printing i+1 next to element number i? It's confusing. Print i and x[i] (as above).

Random number generation

rand() generates a random integer between 0 and RAND_MAX. On many platforms, RAND_MAX is 32767. Here, you're storing this into a floating point array, so it looks like you want to generate a random floating-point number. This is a difficult problem, and the solution depends on what distribution you want. On unix/POSIX platforms, there is a function drand48 which generates a random floating-point number in the range [0,1] with a uniform distribution. This issue is unrelated to the meat of your program, so you may be content with generating a random integer with rand().

A second issue is that rand() and friends use a pseudo-random number generator. This means that they will always return the same sequence of numbers if they are initialized in the same way. Initializing a random generator is called seeding it, and the function to seed the PRNG is srand(int). If you never call that function, your program will always fill the array with the same values. A common way of initializing the PRNG for testing is srand(time(NULL)); this will make your program use different values every second (on most machines).

Random number generation is a fairly complex topic; I recommend reading the clc FAQ, questions 13.15–13.20, especially Each time I run my program, I get the same sequence of numbers back from rand() and How can I generate floating-point random numbers?.

For the time being, add srand(time(NULL)); near the beginning of your main() function, and fix the call to rand().

Input validation

You read an integer with scanf. This function is convenient for quick tests but tricky to use correctly. It's difficult to react properly if the user enters garbage.

Since num should be a positive integer, make it an unsigned type. In C99, make it size_t and call scanf("%z", &num). In C89, make it unsigned long and call scanf("%lu", &num).

Perform at least some token validation of num. The number should be positive (if you want to allow 0, you need to add a special case to some of your calculations). And it must not be larger than the size of the array.

scanf("%lu", &num);
if (num == 0 || num >= sizeof(x)/sizeof(x[0])) {
 printf("Invalid number of elements, goodbye.\n");
 return EXIT_FAILURE;
}

sizeof(x)/sizeof(x[0]) (which can also be written sizeof(x)/sizeof(*x)) is an idiom to denote the number of elements in the array x: it's the size of the array divided by the size of one element. Note that this only works if x is an array, not if x is a pointer. Pointers to arrays don't contain any indication of the size of the array, and the size needs to be specified separately (hence your arr_count argument when you pass a pointer to the array to the various functions).

• Lundin: %lf was introduced as a valid specifier for double in C99. So %lf is universal.(C99 7.24.2.2.11)
• rahmu: What's a better way than scanf to read user input?

Answer 3

Capture the seed returned from time(NULL) into a variable and use the same value to seed rand. This prevents you from making unnecessary calls to time(NULL) thus making it faster, and also has the added bonus that it allows you to do controlled tests by assigning the seed to a fixed value (receive the same 'random' numbers in each run).

• Gilles 'SO- stop being evil': It's not so much that the calls to time(NULL) are necessary, rather they don't make any sense (rand doesn't take any argument, and srand doesnt' return a random number).
• Neil: Using a seed allows you to test your program with the same random numbers which is reason in of itself.

Answer 4

I don't believe that you are using rand() correctly. rand() takes no parameters. You may be thinking of srand(unsigned *seed), which you should call once at the beginning, before calling rand().

Also, you should #include <stdlib.h> for rand().