![]() |
|
![]() |
|
|
Thread Tools | Display Modes |
|
|
#1 |
|
Programmer
Join Date: Jun 2005
Posts: 68
Rep Power: 4
![]() |
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. |
|
|
|
|
|
#2 |
|
Professional Programmer
Join Date: Jan 2006
Location: Ontario, Canada
Posts: 380
Rep Power: 3
![]() |
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.
__________________
I am Addicted to Linux! |
|
|
|
|
|
#3 |
|
Expert Programmer
Join Date: Sep 2004
Location: Ontario, Canada
Posts: 579
Rep Power: 5
![]() |
Too many comments for such a simple program methinks.
I didn't see a need for math.h or stdlib.h
__________________
Johnny was a chemist's son but Johnny is no more, for what Johnny thought was H2O was H2SO4 |
|
|
|
|
|
#4 |
|
Programming Guru
![]() Join Date: Jun 2005
Location: Adelaide, South Australia
Posts: 1,260
Rep Power: 5
![]() |
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. |
|
|
|
|
|
#5 | |
|
Programmer
Join Date: Jun 2005
Posts: 68
Rep Power: 4
![]() |
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:
#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? |
|
|
|
|
|
|
#6 |
|
Professional Programmer
Join Date: Jan 2006
Location: Ontario, Canada
Posts: 380
Rep Power: 3
![]() |
Well there is no need to comment if statements, they explain them selves.
__________________
I am Addicted to Linux! |
|
|
|
|
|
#7 | |
|
Programming Guru
![]() Join Date: Jun 2005
Location: Adelaide, South Australia
Posts: 1,260
Rep Power: 5
![]() |
Quote:
|
|
|
|
|
|
|
#8 |
|
Hobby Coder
Join Date: May 2006
Posts: 59
Rep Power: 0
![]() |
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 |
|
|
|
|
|
#9 |
|
Programmer
Join Date: Jun 2005
Posts: 68
Rep Power: 4
![]() |
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. |
|
|
|
|
|
#10 | |
|
Newbie
Join Date: Aug 2006
Location: Alabama
Posts: 26
Rep Power: 0
![]() |
Quote:
|
|
|
|
|
![]() |
| Bookmarks |
| Currently Active Users Viewing This Thread: 1 (0 members and 1 guests) | |
| Thread Tools | |
| Display Modes | |
|
|
Similar Threads
|
||||
| Thread | Thread Starter | Forum | Replies | Last Post |
| Language display in program | Prm753 | C++ | 3 | May 30th, 2006 6:45 PM |
| Creating a program to test a program | sixstringartist | C | 8 | Jan 21st, 2006 2:15 PM |
| vb6 to vb.net | bl00dninja | Visual Basic .NET | 2 | Sep 24th, 2005 2:55 PM |
| Exception thrown on program exit in debug mode -- Win32 | uman | C++ | 4 | Jun 4th, 2005 5:28 AM |
| airport Log program using 3D linked List : problem reading from file | gemini_shooter | C++ | 0 | Mar 2nd, 2005 5:12 PM |