Another C Programming question

[Raven]

I am running into a problem with a little C program I am writing for school. This program gets a number of integers from a database, and stops doing so when the integer is -1.

It then displays the largest value, the smallest value, the sum, and average of the integers.

I have nested if statements in a do/while loop, but my problem is it keeps reading the smallest value as -1 (despite -1 being the "while" condition). Here is my code....

# include

int main()
{
int Largest, Smallest, Sum = 0, Num;
float Average, Count = 0;

scanf ("%d,&Num);
Largest = Num;
Smallest = Num;
Sum = (Sum + Num);
Count++;

do
{
scanf ("%d,&Num);
Sum = (Sum + Num);
Count++;
if (Num > Largest)
{
(Largest = Num);
}
else if (Num < Smallest)
{
(Smallest = Num);
}
}
while (Num != -1);

Average = (Sum/Count);

and then my printf's which I'll leave out here....

return 0;
}


prints this-
Pos integers: 27
Largest Value: 1234
Smallest Value: -1
Sum: 6557
Average: 242.85

Since -1 should terminate the do/while loop, why is it being assigned as the value of "Smallest", and how can I correct this?

Thanks

[HeavyReign]

Try this:

int main()
{
int Largest, Smallest, Sum = 0, Num;
float Average, Count = 0;

scanf ("%d,&Num);
Largest = Num;
Smallest = Num;

do
{
Sum = (Sum + Num);
Count++;
if (Num > Largest)
{
(Largest = Num);
}
else if (Num < Smallest)
{
(Smallest = Num);
}
scanf ("%d,&Num);
}
while (Num != -1);

Average = (Sum/Count);

...

return 0;
}

I took out this on top because the loop will handle it:

Sum = (Sum + Num);
Count++;

Then I moved the scanf ("%d,&Num) to the end of the loop. This will cause the loop to exit as soon as the -1 is read in without processing it. Under the previous setup, it was also being included in the sum and count.

[Fido]

Originally posted by Raven
Since -1 should terminate the do/while loop, why is it being assigned as the value of "Smallest", and how can I correct this?

Welcome to the joys of computer programming - you're getting mad at the computer, yet its doing exactly what you're telling it to do.

do 
{
    scanf ("%d,&Num);
    Sum = (Sum + Num);
    Count++;
    if (Num > Largest)
    {
        (Largest = Num);
    } 
    else if (Num < Smallest)
    {
        (Smallest = Num);
    }
} while (Num != -1);

The while condition is checked right where it is in your code. In other words, you're telling it to process everything then check if the value is -1. You really want it to check before all of the processing:

scanf ("%d," &Num);
while (Num != -1)
{
    if (Num > Largest)
    {
         Largest = Num;  
    }
    if (Num < Smallest)
    {
        Smallest = Num;
    }
    Count++;
    Smallest = Smallest + Num;
    scanf ("%d," &Num);
}

Also note that I removed the else from the code. If your database should happen to have only 1 valid entry, then it shouldbe the max and the min value, so you need to check it every time through.

[wbonnell]

Without looking too deeply, remember, a "do" loop doesn't evaluate the conditional until AFTER the block has been executed. On the other hand, a "while" loop evaluates the conditional BEFORE execution. Is it possible that -1 is assigned to smallest BEFORE you evaluate it?

[dacman]

Bah, amateurs! What happens if -1 is the only entry? Smallest still gets assigned -1!

Raven--is this a HS class, college, self-taught??? If you're doing this for credit, you need to account for special cases like no entries, 1 entry, 1 entry=-1, etc.

[dacman]

Dola, heres a quick and dirty fix:

Change this line (new stuff in bold):
else if (Num < Smallest && Num!=-1)

[Raven]

OK, I used HeavyReign's code, but took out the else like Fido said.

Results:
Smallest Integer is now 1 (not -1). Sum (6558) and Average (252.23) are readjusted.

WB, it was evaluating -1 before terminating. Thats reflected in the Smallest, Sum and Average changing.

Dac, it is a college class.

From the handout "The data file should contain all positive integers. The last value in the file should be -1. This is the sentinel value that signals the program to stop getting integers as input."

So I'm not sure if that means it should or shouldn't include -1.

[dacman]

On 2nd thought, that still leaves problems -- here's a better fix

while(scanf ("%d,&Num)){
if(Num==-1) {
break;
}
.......(all the other stuff)
}

This will exit on -1 or illegal input. Some older-type programmers won't like the break in a while loop, but it's no longer considered bad programming practice like it was 10+ years ago (well, not by the folks who taught me, anyway).

[dacman]

Originally posted by Raven
So I'm not sure if that means it should or shouldn't include -1.

That would mean no, do NOT include the -1.

I'm guessing its a 100-200 level class, so you may not win extra points (or even brownie ones) for having good (i.e. error checking) code. Still good to learn, though.

[dacman]

Count should probably just be an int. Not sure how nitpicky your graders might be, but mine woulda docked me for that.

[Raven]

He told us to float count so that we don't lose data when computing the average (sum/count). Though floating count is still causing me to get a warning message.

[sabotai]

"This will exit on -1 or illegal input. Some older-type programmers won't like the break in a while loop, but it's no longer considered bad programming practice like it was 10+ years ago (well, not by the folks who taught me, anyway)."

Breaking like this should be avoided , but sometimes it just offers up the best solution.

(FWIW, I would suggest breaking out like this too in this situation)

[dacman]

Originally posted by Raven
He told us to float count so that we don't lose data when computing the average (sum/count). Though floating count is still causing me to get a warning message.

Sum is an int, hence the warning....edit: (float) = (int)/(float) ... hmm on second thought, I'm not sure why you'd get a warning for that. Maybe if average were a double, it'd go away?

[Raven]

This guy is older than old, so I should probably avoid the break.

It is working perfect now, other than the warning message (multi line string literals deprecated) when compiling.

Im trying to correct that now.

[Fido]

Originally posted by dacman
Bah, amateurs! What happens if -1 is the only entry? Smallest still gets assigned -1!

I wouldn't call it amaturish, I'd call it not wanting to hand him a completed homework assignment.

And to defend my amaturishness, I origanlly worte my sample as a while loop instead of a do loop (while is better suited here), but changed it back to better model his original post, and I ran out of time to modify it (build was complete so I had to get back to work)

There's lots of other stuff he shoudl be doing in it. Smallest and Largest shoudl be initialized to some arbitrary values (INT_MAX for Smallest and some negative value for largest). They're random memory values now.

Also there should be data validation code. Even though the spec says the input will be in a specific range does NOT mean that the user will follow the same rules (never trust a user).

There should also be a check to see if count is 0 before computing the average.

Fact is, Raven's just learning (probably a 100 level class) so why flood him with stuff he should be doing that is unrelated to his original question?

[Fido]

Originally posted by Raven
This guy is older than old, so I should probably avoid the break.

It is working perfect now, other than the warning message (multi line string literals deprecated) when compiling.

Im trying to correct that now.

Check to make sure youe open/close quotes match up:

scanf ("%d,&Num);

[Raven]

He specifically said to float average and count

and said he wanted average to be 2 decimal values.

tried setting average as a double, but still getting the warning.

BTW, thanks a lot to all of you guys for your help

[Raven]

Just checked, and all my quotes matched up. That was a typo in my post, not the program, Fido.

[dacman]

Originally posted by Fido
Fact is, Raven's just learning (probably a 100 level class) so why flood him with stuff he should be doing that is unrelated to his original question?

The amateurs comment was totally in jest --- I mention it because I had to deal with it in my 100 level class. Then again mine was an only CS majors class---Raven's MMV.

[Anrhydeddu]

Some older-type programmers won't like the break in a while loop, but it's no longer considered bad programming practice like it was 10+ years ago (well, not by the folks who taught me, anyway).

Uh-huh. Another education trend succumbing to lower standards.

[Bonegavel]

For what it is worth, this:

Sum = (Sum + Num);

can be written as:

Sum += Num;

This is one of the many beautiful things that c++ gives us. Use it. :-)