Here is my code as of now, as well as an input file and my output. I
am aware that I've got more error proofing to do, but this is what I
have right now.
You have not taken some of the advice already given but, what the
hell, lets have another go...
#include <stdio.h>
#include <stdlib.h>
int main(int argc, char *argv[])
{
/* Variable Declarations */
No need for comments like this. If the reader needs it, they will get
lost very soon. If they don't who is it for?
const int arraylength = 5000;
const int exiterrorcode = -1;
C already has name for this: EXIT_FAILURE. It is standard and already
defined (I know here you got your version from and it is not the worst
thing in the course notes, but it is still much worse than using
EXIT_FAILURE).
int count = 0;
int condition = 1;
float* precipvals = (float*)NULL;
float* precip3hrs = (float*)NULL;
long* date = (long*)NULL;
None of the casts in the program are required (the same is true, I
think, for all the casts in your course notes as well) and they just
serve to confuse the reader.
fpin = fopen(argv[1],"r");
fpout = fopen(argv[2],"w");
if(fpin == NULL || fpout == NULL) {
printf("Insufficient arguments\n");
exit(exiterrorcode);
}
It is too late by this time. You could write this:
fpin = argc > 1 ? fopen(argv[1], "r") : NULL);
fpout = argc > 2 ? fopen(argv[2], "w") : NULL);
and then your test will work but, really, you should report unopenable
files as distinct from not enough arguments.
/* Array Allocation */
precipvals = (float*)malloc(sizeof(float) * arraylength);
precip3hrs = (float*)malloc(sizeof(float) * arraylength);
date = (long*)malloc(sizeof(long) *
arraylength);
the form: 'date = malloc(arraylength * sizeof *date);' has so much
going for it, I can't see why it not morewidely used!
...and you know you need to test the results.
/* Calculation Section */
while(condition != EOF) {
condition = fscanf(fpin,"%ld,%f", &date[count],
&precipvals[count]);
condition will never get to EOF if there is an error in the input
format. fscanf can return 1 or 0 and will continue to do so for ever
if the input is malformed. It is much better to test for everything
be hunky-dory:
while (count < array_length &&
fscanf(fpin, "%ld,%f", &date[count], &precipvals[count]) == 2) {
Note I have also checked count (first). You must not read data into
storage that does not exist.
if(count == 0) {
precip3hrs[count] = precipvals[count];
}
if(count == 1) {
precip3hrs[count] = precipvals[count] + precip3hrs[count-1];
}
if(count > 1) {
precip3hrs[count] = precipvals[count] + precipvals[count-1] +
precipvals[count-2];
}
count++;
}
Personally, I would read the data, and then do the partial sums.
/* Output section */
for(count = 0; count < arraylength; count++) {
if(!(precipvals[count] == 0 && precip3hrs[count] == 0)) {
if(count == 0) {
printf("One hour precipitation: %2.2f Three hour precipitation
%2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count == 1) {
printf("One hour precipitation: %2.2f Three hour precipitation
%2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 1) {
printf("One hour precipitation: %2.2f Three hour precipitation
%2.2f\n",precipvals[count],precip3hrs[count]);
}
}
}
I can't see the point of all those tests. You do the same thing in
every case. If you *do* want to do thing differently for different
counts, I'd put the special cases outside the loop and run the loop
from count = 2.
/* Write to file */
for(count = 0; count < arraylength; count++) {
if(!(precipvals[count] == 0 && precip3hrs[count] == 0)) {
if(count == 0) {
fprintf(fpout,"One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count == 1) {
fprintf(fpout, "One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
if(count > 1) {
fprintf(fpout, "One hour precipitation: %2.2f Three hour
precipitation %2.2f\n",precipvals[count],precip3hrs[count]);
}
}
}
Ditto.
fclose(fpin);
fclose(fpout);
/* Deallocation */
free(precipvals);
precipvals = (float*)NULL;
free(precip3hrs);
precip3hrs = (float*)NULL;
date = (long*)NULL;
free(date);
That must be a typo. Surely you want to free(date) then set it to
NULL? Personally, I would just write:
free(precipvals);
free(precip3hrs);
free(date);
