problem 1.13 The C programming language
joag
I'm doing things wrong, I'm really new to programming (two month or so)
so please I need all the guidance possible in order to improve.
Best Regards,
joag
Barry Schwarz
wrote:
For only 85 lines, it is better to include the code in your message.
Not that it is an error but why initialize an integer to 0.0 (line
21)?
Unless you are using C99, you need to put your declarations (line 21)
before your statements (line 20).
In your while loop, nword is indexed by the number of characters in
the current completed word (line 26). Do you ever expect to have a
word with 15000 characters in it (line 21)?
Let's assume the input was "abcd efg hijk$" where the $ represents end
of file. Your while loop will recognize abcd and increment nword[4]
(line 26) and nw (line 27), then recognize efg and increment nword[3]
and nw again. Unfortunately, it will exit (line 23) before it
finishes processing hijk.
At this time nw is 2 and two elements of nword have non-zero values.
Your first for loop (line 39) will process only the first three
entries of nword, all of which are zero. It never sees elements 3 and
4. You probably meant to use counter (which actually holds the max
word length +1 and therefore the last element +1 of nword which is
non-zero) and not nw as the loop limit. If you do use counter, change
the <= to <.
Why does this loop process nword[0] which gets incremented only if
there are two or more consecutive "spaces"? Why is the second printf
(line 41) which is not part of the loop indented as if it is?
Your second for loop (line 43) has the same limit problem.
Your fourth for loop (line 55) prints any non-zero value. This
guarantees that you build the histogram with low frequencies on top
and higher frequencies running down. If you test nword[j] against i
instead of 0, you can print the histogram in "normal" orientation.
Your first else if (line59) tests a condition which must be true. You
can replace it with a simple else.
--
Remove del for email
joag
the code is doing practically the same, I tested it against this text
(#### is just for delimiting the text):
###### In your while loop, nword is indexed by the number of characters in
the current completed word (line 26). Do you ever expect to have a
word with 15000 characters in it (line 21)? ######
1 1 9 3 8 6 2 2 0 1 2
and as you see I never print position zero but instead 1 2 and so on,
position one (one repetition) is printed well, position two (9
repetitions) is printed well, position three (3 repetitions) so it is
printed well too, but since there something goes wrong, if you see there
is eight asterisks but they are not aligned.
9 | * * * * * * * * * *
8 | * * * * * * *
7 | * * * *
6 | * * *
5 | * * *
4 | * * *
3 | * *
2 | * *
1 | *
|________________________
1 2 3 4 5 6 7 8 9 10 11
Barry Schwarz
wrote:
>I follow your instruction but the part for changing 0 to i in line 55 and
>the code is doing practically the same, I tested it against this text
>(#### is just for delimiting the text):
You were asked to post your code. You also need to learn to quote
what your are responding to.
I did not say you should just change the line of code. You need to
rethink how you perform the test and what you do with each possible
result.
snip out of context discussion and useless sample output
joag
printed in the right order from bottom to top (you know what I mean), I
think it need more testing but by far all text I use as input produce a
good histogram. The code was working fine as you can see I had just
problem with spacing on lines 37 40 and 42 as you can see now it has
double space instead single and this did the trick.
this is the code:
/*
* File: histogram.c
* Author: joag
* Created on march 3, 2010, 15:28
* S. America Pacific Time (Panama, GMT-05:00)
*/
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
/*
*
*/
int main(int argc, char** argv)
{
int c, i, j, nc, counter, yaxis;
int nword[15000] = {0.0};
nc = counter = yaxis = 0;
while ((c = getchar()) != EOF){
++nc;
if (c == ' ' || c == '\n' || c == '\t' || c == EOF ){
++nword[nc - 1];
if (counter < nc)
counter = nc;
nc = 0;
}
}
for (i = 0; i < counter; ++i)
if (nword[i] > yaxis)
yaxis = nword[i];
for (i = yaxis; i > 0; --i){
printf("%4d | ", i);
for (j = 1; j < counter; ++j){
if (nword[j] >= i)
printf("* ");
else
printf(" ");
}
printf("\n");
}
printf(" |");
for (i = 0; i < counter; ++i)
printf("___");
printf("\n ");
for (i = 1; i < counter; ++i)
printf("%2d ", i);
printf("\n\n");
return (EXIT_SUCCESS);
}
Flash Gordon
> Sorry about that, I fixed line 39 (now is 39) and the histogram is
> printed in the right order from bottom to top (you know what I mean), I
> think it need more testing but by far all text I use as input produce a
> good histogram. The code was working fine as you can see I had just
> problem with spacing on lines 37 40 and 42 as you can see now it has
> double space instead single and this did the trick.
>
> this is the code:
>
> /*
> * File: histogram.c
> * Author: joag
> * Created on march 3, 2010, 15:28
> * S. America Pacific Time (Panama, GMT-05:00)
> */
>
> #include <stdlib.h>
> #include <stdio.h>
> #include <string.h>
>
> /*
> *
> */
>
> int main(int argc, char** argv)
> {
> int c, i, j, nc, counter, yaxis;
> int nword[15000] = {0.0};
As previously mentioned, it would make more sense to use 0 in the above
line rather than 0.0
> nc = counter = yaxis = 0;
These variable could be initialised on declaration rather than a
separate initialisation here.
> while ((c = getchar()) != EOF){
> ++nc;
> if (c == ' ' || c == '\n' || c == '\t' || c == EOF ){
c will never be EOF here, since you terminate the loop if it is. Which
also means you don't increment the nword element for the last word entered.
> ++nword[nc - 1];
What if nc is more than 15000? I would probably use something rather
smaller than 15000 as the array size, and put an error trap in to catch
overly long words.
> if (counter < nc)
> counter = nc;
> nc = 0;
> }
> }
>
> for (i = 0; i < counter; ++i)
I would initialise yaxis here rather than at the top of main, i.e.
for (i = 0, nword = 0, i < counter, i++)
> if (nword[i] > yaxis)
> yaxis = nword[i];
>
> for (i = yaxis; i > 0; --i){
> printf("%4d | ", i);
> for (j = 1; j < counter; ++j){
> if (nword[j] >= i)
> printf("* ");
> else
> printf(" ");
> }
> printf("\n");
> }
>
> printf(" |");
>
> for (i = 0; i < counter; ++i)
> printf("___");
>
> printf("\n ");
>
> for (i = 1; i < counter; ++i)
> printf("%2d ", i);
What will happen to your formatting if you need more than two digits?
> printf("\n\n");
Why two new lines?
> return (EXIT_SUCCESS);
Simpler (and with the same meaning) would be
return 0;
> }
--
Flash Gordon
joag
also added a topic on my page www.nixheiser.org (guides - problem 1.13
section) so other people can reach the same topic and comment about it
there because there is a little forum for comments; in the rest of the
month I'll try to do what follows in the book (The C programming
language by K&R). I've been told by other people to start with this
book and I never did this and now I see why people recommend it, it is
great ^n book.
joag
Flash Gordon
> I'll try the next week to update the program with your suggestions, I
> also added a topic on my page www.nixheiser.org (guides - problem 1.13
> section) so other people can reach the same topic and comment about it
> there because there is a little forum for comments;
You will find a number of solutions to various of the exercises at
http://clc-wiki.net/
Obviously do each exercise yourself before looking at other peoples
solutions.
> in the rest of the
> month I'll try to do what follows in the book (The C programming
> language by K&R). I've been told by other people to start with this
> book and I never did this and now I see why people recommend it, it is
> great ^n book.
It is a good book.
--
Flash Gordon