Programming Forums - Help debug (short program)

Programming Forums (http://www.programmingforums.org/forumindex.php)

- C (http://www.programmingforums.org/forum60.html)

- - Help debug (short program) (http://www.programmingforums.org/showthread.php?t=11234)

Help debug (short program)

Hello everyone, some of you may recall my screen name, or even less likely, my modest contributions to this forum, but after about a year and some more programming classes I have returned, eager to learn more.

EDIT: The initial reason for this post was to recieve help debugging a portion of my homework, but in getting it ready and forming my question for this post, I discovered what I was doing wrong. So I will now like to post it, so that I may recieve feedback on my work. Structure/efficiency/logic/etc.

The following code was created using vi on the unix operating system and was compiled and run with gcc -g -Wall -ansi

/*********************************

* 

* Programmer:Chris Cadwallader

*

* Assignment:HW 1 Q: 2

*

* Date Completed:8/31/2006

*

* Program's funcion:

* To take user input of floats until it 

* reads -999. Then it takes those floats

* and calculates and displays the average

* and maximum value.

**********************************/



#include <stdio.h>

#include <stdlib.h>

#include <math.h>



#define TERMINATE -999



void getFloats(float*, float*);

void printResults(float,float);



int main()

{

  /* Declarations */

  float valueAverage = 0; /* Average of input values */

  float maxValue = 0;     /* Maximum input value */



  getFloats(&valueAverage, &maxValue);

  printResults(valueAverage, maxValue);

  return(0);

}/* Main */





/******************************

*

* Programmed by:Chris Cadwallader

*

* Function Name:getFloats

*

* Arguments:address of valueAverage and maxValue

*

* Function description:

* Takes user input of floats and calculates

* their average and determines the maximum

* input value.

*

*****************************/

void getFloats(float *valueAverage, float *maxValue)

{

  /* Declarations */

  int divider = 2; /* value to average 2 values */

  float input;     /* input value */



  /* Statements */



  printf("Please input floats to average. When finished enter -999 to terminate.");



  scanf("%f", &input);

  *valueAverage = (input + input)/divider;

  

  while(input != TERMINATE)

  {

    scanf("%f", &input);

    if (input != TERMINATE)

    { 

      *valueAverage = (*valueAverage + input)/divider;

    }/*if*/ 

    if (*maxValue < input)    

    {

      *maxValue = input;

    }/*if*/

  }/*while */

  return;

}/* getValues */





/******************************

*

* Programmed by:Chris Cadwallader

*

* Function Name: printResults

*

* Arguments:float valueAverage //Average of values

*         float maxValue //Maximum input value

*

* Function description:

* Prints the Results of the previous*

* functions. Output is the average and

* maximum value of input.

*****************************/

void printResults(float valueAverage, float maxValue)

{

  printf("The average value is %f\n", valueAverage);

  printf("The maximum value is %f\n", maxValue);

  return;

}/* printResults */

Any feedback is greatly appreciated. Oh, and Im relatively new at this so be gentle.

I haven’t even really looked at what the code does, but one thing I noticed is you have a lot of useless comments all over the place that makes your code ugly and harder to read. I might take a closer look at it a bit later tonight, but definitely get rid of all those useless comments.

Too many comments for such a simple program methinks.
I didn't see a need for math.h or stdlib.h

In addition to proceeding comments.

1) Last I checked, (input + input)/divider can be simplified to simply "input" when divider has a value of 2.

2) Never use a check for equality between floating point values as a condition in an if statement or a while loop. Because of finite numerical precision in floating point types, which means values may not be represented exactly, that test may yield false positives and false negatives (eg you may think you entered a value of -999 but the value stored is slightly different).

3) The average of a set of values is defined as the sum of those values divided by the number of those values. Your computation of valueAverage does not compute the average.

4) Always check the return value of scanf() to determine if it successfully read a value. Just checking the value read does not achieve that.

5) It is probably a good idea for getFloats() to reinitialise it's arguments (particularly *maxValue, but probably *valueAverage depending on how you address my point 3 above) to zero. At the moment, if you call the function twice, you are relying on the caller to do that. That will certainly mean any value you compute of valueAverage is incorrect (even if you do a valid calculation of it).

6) In practice, you might want the number of data values read by getValues() to be passed back to the caller. A simple way to do this is to change the return type of that function and ... return it.

7) Use a consistent naming convention for variables and function argument names (and function names if you can). One variable named maxValue and another named valueAverage is not a consistent naming convention. That makes it unnecessarily more difficult, in practice, for someone (eg you in a few months) to understand what the program is doing.

The comments are required for the project (headers that is) as for the rest of the comments, I havent got a feel for what the professor is expecting so I commented all declarations and anytime I had doubt.

I threw the sdtlib and math.h in there from habit.

Quote:

Last I checked, (input + input)/divider can be simplified to simply "input" when divider has a value of 2.

Thanks grumpy, I didnt see how senseless that line was.

#3.... well crap.... I blew the logic on that one. ... crap...

What do you mean by checking if scanf() successfully read a value? Thats something we have never done so could you explain a bit?

Well there is no need to comment if statements, they explain them selves.

Quote:

Originally Posted by sixstringartist

What do you mean by checking if scanf() successfully read a value? Thats something we have never done so could you explain a bit?

Look at the documentation for scanf(). The return value is the number of values successfully read. In your example, each call of scanf() is attempting to read one value (identified in the format string by %f and the corresponding argument). So, if your calls to scanf() return anything other than 1, they have not succeeded.

Your program is basically well done, but in fairness, it seems:

1) Not to average anything. That is, it doesn't have two values that it's averaging, it's taking just ONE value and "averaging" that value - which is naturally itself. You need to average TWO or more distinct values (although some or all may have the same value, they are separate entries).

2) You'll need some data structure to hold the first value, while the second value is being input.

Good luck!

Adak

Yeah, the objective of averaging in this assignment was never achieved and I see why now. oh well.

I should add that we were instructed not to use arrays on this particular assignment.

Quote:

Originally Posted by sixstringartist

I should add that we were instructed not to use arrays on this particular assignment.

Arrays are not needed when averaging. Only two vars are needed (1) for the total and (2) for the number of numbers.