Click to See Complete Forum and Search --> : Why doesnt this code work


Eric
June 7th, 1999, 12:19 AM
Thank You for reading this.
I am new to c++ and I wrote this code to see if it will work but it doesnt. could you please tell me why it doesnt work or is there a easier way of doing it. I wanted to make a function to get the results but I am not sure how to use functions yet.
The code is supposed to get competitors scores and average them. Then all the competitors below average are to be assigned -999 and then all of them to be printted out.
But it doesnt work.
Please tell whats wrong or how to use functions to get the results.
I am new to this so please explain in easy language.
I thank you in advance
Eric

#include <iostream.h>
#include <stdio.h>
main()
{

int num[5];
int sum;
float average;

for( int i=0; i<5; ++i )
{
cout<<"Enter competitors score: ";
cin>>num[i];
}

for( int x=0; x<5; ++x )
{
sum += num[x];
}

average = sum/5;

for( int y=0; y<5; ++y )
{
if ( num[y]<average )
num[y]= -999;
}

for( int z=0; z<5; ++z )
{
if ( num[z]!= -999 )
cout<<"These competitors scored above average: "<<num[z];
cout<<"\n";
}

for( int c=0; c<5; ++c )
{
if ( num[c]== -999 )
cout<<"These competitors scored below average: "<<num[c];
cout<<"\n";
}

cout<<"End of program";

return 0;
}

June 7th, 1999, 12:33 AM
int num[5];
int sum;
float average;
////cut

average = sum/5;

////////////////
if the sum is 49, then average is 9.
if you want a float, replace
int sum with float sum.
the difference is
49/5 = 9 and 4/5
49.0/5 = 9.8

if you don't declare one or the other
var as float, value of / is an integer

I don't know but maybe
average = (float)sum/5
can be done, we say to the compiler to
convert sum in float before doing /

Eric
June 7th, 1999, 12:40 AM
Thank you. So simple hehehehe. I was close but for a simple (float error ) hehehe.
I thank you again.

Roger Allen
June 7th, 1999, 03:40 AM
You also have a slight bug in your reading in of numbers :

You need to change
cin>>num ;
to
cin>>num(i) ; // should be square brackets[] but code guru does not show it in the posted post?
as you are reading all the values into the same location in the num array.


Roger Allen

ric
June 7th, 1999, 06:12 AM
If I have written this code I would not laugh that much at the double precision error, but look more on style and code errors and senseless stuff. The error cin >> num instead of cin >> num(i) is more important than some stupid double precision truncating.
1. Why do use so much ints on the stack -- x,y,z,c. They all are alive only in the loops so use just i for all.

2. Why this cout << "Enter competitors score: "; is in the loop? Are sure you want it on every iteration? Because you tell the user to enter the competitors scores, but when he enters one competitor scores he get the message again "Enter competitors score: ". And he got confused. Better do it this way:

cout<<"Enter competitors score: ";
int i(0);
while(i<5)
{
cout << ++i << ". ";
cin>>num(i);
}

3. The sum is never defined to be 0. So at the point you are adding to it, it is some random number.

4. Why do you need two loops to print the scores? If you have had 100 option would you made a 100 loops? Print everything you need in a single loop. Like:

for( i=0; i<5; i++ )
{
if ( num(i)!= -999 )
{
cout<<"These competitors scored above average: " << i << "There scores: " << num(i);
cout<<"\n";
}
else
{
cout<<"These competitors scored below average: << i;
cout<<"\n";
}
}

You do not have to print -999 from num(i) still you know that it is -999;

My advice first think, then write code.

Regards,
ric

Dave Lorde
June 7th, 1999, 06:59 AM
> 2. Why this cout << "Enter competitors score: "; is in the loop?

Because it is prompting for the score of a competitor... It's good practice to provide a prompt for every required input.

Better this:
Enter competitors score: 22
Enter competitors score: 34
Enter competitors score: 43
Enter competitors score: 56
Enter competitors score: 19

than this:

Enter competitors score: 22
34
43
56
19

> 4. Why do you need two loops to print the scores? If you have had 100 option
> would you made a 100 loops? Print everything you need in a single loop. Like:

You miss the point. If you use a single loop as you suggest, the competitor numbers for above and below average are mixed, so the prompts don't make sense, e.g.:

These competitors scored below average: 1
These competitors scored below average: 2
These competitors scored above average: 3
These competitors scored above average: 4
These competitors scored below average: 5

> You do not have to print -999 from num(i) still you know that it is -999;

You don't really need to use -999 at all...

> My advice first think, then write code.

Wise words :-)

This is how I would do it:


#include <iostream.h>
#include <stdio.h>

const int TotalEntries = 5;

main()
{
int num[TotalEntries];
int sum(0);
float average(0.0);

for( int i = 0; i < TotalEntries; ++i )
{
cout << "Enter competitors score: ";
cin >> num[i];
sum += num[i];
}

average = (float)sum / 5;
cout << endl << "The average is: " << average << endl;

cout << endl << "These competitors scored above average: " << endl;
for( i = 0; i < TotalEntries; ++i )
{
if (num[i] > average)
cout << i + 1 << endl;
}

cout << endl << "These competitors scored below average: " << endl;
for( i = 0; i < TotalEntries; ++i )
{
if (num[i] < average)
cout << i + 1 << endl;
}

cout << endl << "End of program" << endl;
return 0;
}




Dave

ric
June 7th, 1999, 09:03 AM
I never said that there must not be prompt. My solution was:

Enter competitor scores:
1. 989
2. 78
3. 7898

rather than:
Enter competitors scores:
234
2343
234
234

It is stupid to prompt for the competitorS scores and to get only one competitor scores.

The two loops are the most stupid thing I ever heard of. If you have about 1000000 items in this array, you will loop them twice, ah?

This is faster and wiser:

for(int i(0);i<5;i++)
if(num(i)>avarage) "above\n" >> buffer1;
else "below\n" >> buffer2;

cout << buffer1;
cout << buffer2;

Eric
June 8th, 1999, 02:21 AM
Thank you for responding.
I am self teaching myself and I guess I have alot to learn. I will take your advice onboard.
Thank You Again.
Eric

John_Reese
June 8th, 1999, 03:24 AM
Hey man...You need to review your use of arrays and loops...I'll show you what is most obvious to me, but without a compiler I could be typing errors...You'll just have to play with it...



//All corrections will be in CAPS

#include <iostream.h>
#include <stdio.h>
INT main() //You are returning at the end, need return value;
{ //Can also lose the return statement and define as void main()
int num[5];
int sum = 0;
float average = 0; //good idea to initialize these

for( int i=0; i<5; ++i )
{
cout<<"Enter competitors score: ";
cin>>num; //Needs to be num[i], otherwise you just overwrite yourself 5 times.
}

for( int x=0; x<5; ++x )
{
sum += num[x];
}

average = sum/5;

for( int y=0; y<5; ++y )
{
if ( num[y]<average )
num[y]= -999;
}

for( int z=0; z<5; ++z )
{
if ( num[z]!= -999 )
cout<<"These competitors scored above average: "<<num[z];
cout<<"\n";
}

for( int c=0; c<5; ++c )
{
if ( num[c]== -999 )
cout<<"These competitors scored below average: "<<num[c];
cout<<"\n";
}

cout<<"End of program";

return 0;
}





Ok, I don't know if that fixes everything, but it should at least get you headed in the right direction...Good luck...

john simmons

Dave Lorde
June 8th, 1999, 04:05 AM
> It is stupid to prompt for the competitorS scores and to get only one competitor scores

It looks like a matter of semantics. I assumed he was using the singular possesive but omitted the apostrophe (as his usage of 'score' rather than 'scores' implied), I guess you assumed he was using the plural and hadn't made 'score' plural. It's not stupid if you understand English grammar. He could have intended either meaning.

> The two loops are the most stupid thing I ever heard of. If you have about
> 1000000 items in this array, you will loop them twice, ah?

As I recall, the example used a hardcoded 5 items. Hardly a major overhead to iterate them twice. If this was a large-scale 'serious' example, I would code it completely differently from the start. This one looks to me like an exercise where clarity is more important. Your solution is an optimisation. The rule of thumb with optimisation is don't do it unless you encounter a performance problem. Again, it's not stupid unless you believe there's only 'right' way to code it.

> This is faster and wiser:

It's certainly better than your previous suggestion. Whether it's wiser depends on the circumstances.

Your mileage may vary.

Dave

ric
June 8th, 1999, 04:37 AM
OK,
Let's settle this out! Everyone has his own view of world. None is right and none is wrong. None is stupid and none is
smart. Everything depends on the situation. Your solution for this concrete situation is perfect. But the man is still learning
-- let's not learn him to iterate through an array a thousand times to get what he wants. If some "great guru" told him that
it is right to loop twice now, afterwards no one will convince him that it is right if he only has to iterate through the three
names of his grandpa.
No sense to quarrel like children over this.
Regards,
ric

Dave Lorde
June 8th, 1999, 05:29 AM
OK, whatever...

Dave