bill reed

I'm having a problem with the Quiz output of this program. Notice Quiz
1-4 high score and Quiz 5 low score in the output. Why am I getting
these results?

#include <stdio.h>

#define N 5

int main(void)
{
int a[N][N];
int i;
int j;
int sum = 0;
int low;
int high;

for(i = 0; i < N; ++i) {
printf("Enter 5 Quiz grades for Student %d: ", i + 1);
for(j = 0; j < N; ++j) {
scanf("%d", &a[i][j]);
}
}

for(i = 0; i < N; ++i) {
sum = 0;
for(j = 0; j < N; ++j) {
sum += a[i][j];
}
printf("\nStudent %d total score: %d", i + 1, sum);
printf("\nStudent %d average score: %d\n", i + 1, sum / N);
}

for(i = 0; i < N; ++i) {
sum = 0;
low = a[i][j];
high = a[i][j];
for(j = 0; j < N; ++j) {
sum += a[i][j];
if(low > a[i][j])
low = a[i][j]++;
if(high < a[i][j])
high = a[i][j]++;
}
printf("\nQuiz %d average score: %d\n", i + 1, sum / N);
printf("Quiz %d low score: %d\n", i + 1, low);
printf("Quiz %d high score: %d\n", i + 1, high);
}

return 0;
}


Enter 5 Quiz grades for Student 1: 51 52 53 54 55
Enter 5 Quiz grades for Student 2: 61 62 63 64 65
Enter 5 Quiz grades for Student 3: 71 72 73 74 75
Enter 5 Quiz grades for Student 4: 81 82 83 84 85
Enter 5 Quiz grades for Student 5: 91 92 93 94 95

Student 1 total score: 265
Student 1 average score: 53

Student 2 total score: 315
Student 2 average score: 63

Student 3 total score: 365
Student 3 average score: 73

Student 4 total score: 415
Student 4 average score: 83

Student 5 total score: 465
Student 5 average score: 93

Quiz 1 average score: 53
Quiz 1 low score: 51
Quiz 1 high score: 61

Quiz 2 average score: 63
Quiz 2 low score: 61
Quiz 2 high score: 71

Quiz 3 average score: 73
Quiz 3 low score: 71
Quiz 3 high score: 81

Quiz 4 average score: 83
Quiz 4 low score: 81
Quiz 4 high score: 91

Quiz 5 average score: 93
Quiz 5 low score: 4
Quiz 5 high score: 95

Thanks,
Bill

mike wahler

Why are you changing the supplied data?
I haven't looked to see if that's the cause of
the symptom you describe, but it surely will
give wrong answers about the originally supplied
data.

-Mike

mike wahler

Here's an example of how to structure your code
so that tracking down things should be much easier.
(Note: I wrote the 'main()' function *first*, then
when back and wrote functions to satisfy it.)
I think it's largely due to this up-front intentional
'separation of labor' that I got correct output first try. :-)
#include <stdio.h>
#include <stdlib.h>

#define N 5

enum which {LOW, HIGH};

int sum(int *data, size_t items)
{
int result = 0;
size_t i = 0;

for(i = 0; i < items; ++i)
result += data[i];

return result;
}

int avg(int *data, size_t items)
{
return sum(data, items) / items;
}

int bound(int *data, size_t items, enum which hilo)
{
int item = *data;
size_t i = 0;

for(i = 0; i < items; ++i)
switch(hilo)
{
case LOW:
if(data[i] < item)
item = data[i];
break; case HIGH:
if(data[i] > item)
item = data[i];
break;
default:
break;
}

return item;
}

void stats(int (*data)[N], size_t items)
{
size_t i = 0;

for(i = 0; i < items; ++i)
{
printf("%d. total == %d\n", i + 1, sum(data[i], N));
printf("%d. average == %d\n", i + 1, avg(data[i], N));
putchar('\n');
}

for(i = 0; i < items; ++i)
{
printf("%d. average == %d\n", i + 1, avg(data[i], N));
printf("%d. low == %d\n", i + 1, bound(data[i], N, LOW));
printf("%d. high == %d\n", i + 1, bound(data[i], N, HIGH));
putchar('\n');
}
}

int main(void)
{
int grades[N][N] =
{
{51, 52, 53, 54, 55},
{61, 62, 63, 64, 65},
{71, 72, 73, 74, 75},
{81, 82, 83, 84, 85},
{91, 92, 93, 94, 95},
};

stats(grades, N);
return 0;
}

Output:

1. total == 265
1. average == 53

2. total == 315
2. average == 63

3. total == 365
3. average == 73

4. total == 415
4. average == 83

5. total == 465
5. average == 93

1. average == 53
1. low == 51
1. high == 55

2. average == 63
2. low == 61
2. high == 65

3. average == 73
3. low == 71
3. high == 75

4. average == 83
4. low == 81
4. high == 85

5. average == 93
5. low == 91
5. high == 95


HTH,
-Mike

bill reed

Thanks. I think this is right now. At least the results are what I
expect.

for(j = 0; j < N; ++j) {
sum = 0;
low = a[0][j]--;
high = a[0][j]++;
for(i = 0; i < N; ++i) { sum += a[i][j];
if(low > a[i][j])
low = a[i][j]++;
if(high < a[i][j])
high = a[i][j]++;
}
printf("\nQuiz %d average score: %d\n", j + 1, sum / N);
printf("Quiz %d low score: %d\n", j + 1, low);
printf("Quiz %d high score: %d\n", j + 1, high);
}

Bill

bill reed

LOL. I must have run my program dozens of times and each time I
entered these 25 numbers at the prompt.

Quiz 1
average == 71
low == 51
high == 91 etc.


My code and confusion probably misled you but it finally dawned on me
that I was using the student grades (rows) to get the individual
quiz's (column's) average, high and low score. I thought that was what
you meant when you said I was using the wrong data.

Thanks for the code. It'll probably take me a week to figure out how
it works. That's alright, though, I've got plenty of time.

Bill

mike wahler

Given that obvious symmetry of the input, I did
flirt with the idea of just writing a loop to
generate it, but 'cut-n-paste', with a few
commas, thrown in, was faster. :-)

Of course if I was taking the input from outside,
again, I'd write a separate function for that.

Then in main, it's just:

get_data(grades, N);

When you can keep a function down to a small number of
statements like this, it's much easier and more
effective to debug by inserting 'status' outputs
between them, instead of having to wade through
lots of stuff just to determine the best placement
of the status message.

No, I said that when I saw:
if(low > a[i][j])
low = a[i][j]++;
if(high < a[i][j])
high = a[i][j]++;

You're incrementing a[i][j] in two places.
But now that I think about it, if you only
visit those elements a single time, it shouldn't
affect the output. I was reacting to a 'bad smell'
in the code. :-) BTW why do you think you need those '++' there?

It shouldn't. I wrote it specifically as an example
of readable code. I gave no thought to 'efficiency'
or 'elegance' etc. I tried to simply clearly express
to a *human* reader, what the program does.

Note how it's broken up into several simple functions
each doing a single task, with the larger tasks calling
upon and/or combining them as needed.

People actually already think this way, whether you
realize it or not. :-)

Think about e.g. a cooking
recipe. It might say something like:

int main()
{
boil(EGG, 2);
grate(CHEESE, 0.25);
brown(ONIONS, 1.0);
/* etc */
}

It does *not* say:

int main()
{
fill_saucepan(WATER, .75);
r = open_refrig();
remove(r, EGGS, 2);
stove(ON);
/* etc */
}

That second main is the 'boil()' function stored
in your head. You already have 'grate' and 'brown'
functions there too. So the main task is much more
clearly expressed without all the details cluttering
things up, and perhaps even being performed in the
wrong order. Or perhaps the wrong operations being
applied to the wrong data. The refrigerator might get boiled! :-)


Feel free to ask any questions about it. Be sure to
watch for any criticism of it by others, too.

-Mike

b. v ingen schenau

Why are you changing the scores for the first student here?
These two lines should read:
low = a[0][j];
high = a[0][j];

Why are you changing the student scores here?
This line should read:
low = a[i][j];

Why are you changing the student scores here?
This line should read:
high = a[i][j];


Bart v Ingen Schenau
--
a.c.l.l.c-c++ FAQ: http://www.snurse-l.org/acllc-c++/faq.html
a.c.l.l.c-c++ FAQ mirror: http://nullptr.merseine.nu:8080/acllcc++.html
c.l.c FAQ: http://www.eskimo.com/~scs/C-faq/top.html
c.l.c++ FAQ: http://www.parashift.com/c++-faq-lite/

bill reed

I realize now that the increment and decrement operators are
superfluous but, surprisingly, they actually don't change the output
of the program. I was trying to compensate for a different problem
(the wrong way of course) and neglected to remove them.

Karl Heinz warned me about my fly-by-the-seat-of-my-pants method of
programming. I've even stooped to the shotgun method but I'm trying to
do better. Usually, I at least /think/ I have a good reason for doing
what I'm doing.

Let's just be glad that I will never endanger anyone's life, limb, or
loot by any code that I write. :-)

Thanks for catching the errors,
Bill