need some help with this histogram of words program........
 

This is one of the programs from the K&R Ansi C Book that you have to
write I've had a go at it but got kinda stumped here. So here's my
effort so far...... And I'm aware there is a new ISO standard for C
C99 as of March 2000 and I'm getting around to learning and adopting
that as well but my main concern is actually learning the
language....

/* copyright 2011 Ceriousmall. . . .

Program prints a histogram of the length of words in its input */

#include <stdio.h>

#define IN 1 /* inside a word */
#define OUT 0 /* outside a word */

main()
{
char x;
int c, i, n, nchar, state;
int wordlength[13];
int horscale[13], verscale[51];

x = '*';
state = OUT;
n = nchar = 0;

for (i = 0; i < 13; ++i) {
wordlength[i] = 0;
horscale[i] = i;
}
for (i = 0; i < 51; ++i)
verscale[i] = i;

while ((c=getchar()) != EOF) {
if (c == ' ' || c == '\n' || c == '\t')
state = OUT;
else if (state == OUT) /* condition controlls the start */
state = IN; /* of a word */

if (state == IN)
++nchar;
else if (nchar == 1)
++wordlength[1];
else if (nchar == 2)
++wordlength[2];
else if (nchar == 3)
++wordlength[3];
else if (nchar == 4)
++wordlength[4];
else if (nchar == 5)
++wordlength[5];
else if (nchar == 6)
++wordlength[6];
if (state == OUT)
nchar = 0;
}
for (i = 50; i >= 0; --i)
if (wordlength[1] == verscale[i])
for (i = verscale[i]; i >= 0; --i)
printf("%2d| %c\n", verscale[i], x);
printf(" +------------------------------------\n");

for (i = 0; i < 13; ++i)
printf("%3d", horscale[i]);
printf("\n");

return 0;
}

Re: need some help with this histogram of words program........
 

On Feb 2, 5:35*am, Ceriousmall <divadsm...@gmail.com> wrote:
>
> * * * * * * * * if (state == IN)
> * * * * * * * * * * * * ++nchar;
> * * * * * * * * else if (nchar == 1)
> * * * * * * * * * * * * ++wordlength[1];
> * * * * * * * * else if (nchar == 2)
> * * * * * * * * * * * * ++wordlength[2];
> * * * * * * * * else if (nchar == 3)
> * * * * * * * * * * * * ++wordlength[3];
> * * * * * * * * else if (nchar == 4)
> * * * * * * * * * * * * ++wordlength[4];
> * * * * * * * * else if (nchar == 5)
> * * * * * * * * * * * * ++wordlength[5];
> * * * * * * * * else if (nchar == 6)
> * * * * * * * * * * * * ++wordlength[6];
>
You can index by a variable. In fact this is the normal way to use
arrays.

So ++wordlength[nchar];

(You also need to test nchar to make sure it doesn't go above 12 if
you have a 13-element array. Remember the indexes count from zero.)

Re: need some help with this histogram of words program........
 

Ceriousmall <divadsmall@gmail.com> writes:

> This is one of the programs from the K&R Ansi C Book that you have to
> write I've had a go at it but got kinda stumped here. So here's my
> effort so far...... And I'm aware there is a new ISO standard for C
> C99 as of March 2000 and I'm getting around to learning and adopting
> that as well but my main concern is actually learning the
> language....
>
> /* copyright 2011 Ceriousmall. . . .
>
> Program prints a histogram of the length of words in its input */
>
> #include <stdio.h>
>
> #define IN 1 /* inside a word */
> #define OUT 0 /* outside a word */
>
> main()
> {
> char x;
> int c, i, n, nchar, state;
> int wordlength[13];
> int horscale[13], verscale[51];

These "magic numbers" should be given names with a #define or an enum.

> x = '*';
> state = OUT;

When there are only two states I refer to avoid neutral names like
'state' in favour of boolean variables (in C90 these are just int
variables) with a biased name: 'in_a_word' or 'between_words' and so
on. That way, you don't need to name the states.

> n = nchar = 0;
>
> for (i = 0; i < 13; ++i) {
> wordlength[i] = 0;
> horscale[i] = i;
> }
> for (i = 0; i < 51; ++i)
> verscale[i] = i;

You set verscale[i] to i and never change it. That's not a useful
array -- you can always just use the index instead; but then you also
don't use horscale in any useful way. Without any idea what these
array are there for I can't suggest what you should be doing with them.
You don't need either for a simple histogram with horizontal bars.

> while ((c=getchar()) != EOF) {
> if (c == ' ' || c == '\n' || c == '\t')
> state = OUT;
> else if (state == OUT) /* condition controlls the start */
> state = IN; /* of a word */

Look at the last if a bit more. What happens when state is IN? The
effect will be to leave state as IN so you could just have written

if (c == ' ' || c == '\n' || c == '\t')
state = OUT;
else state = IN;

or even

state = (c == ' ' || c == '\n' || c == '\t') ? OUT : IN;

If you take my idea of a boolean rather than a neutral state variable,
you could write:

between_words = (c == ' ' || c == '\n' || c == '\t');

>
> if (state == IN)
> ++nchar;
> else if (nchar == 1)
> ++wordlength[1];
> else if (nchar == 2)
> ++wordlength[2];
> else if (nchar == 3)
> ++wordlength[3];
> else if (nchar == 4)
> ++wordlength[4];
> else if (nchar == 5)
> ++wordlength[5];
> else if (nchar == 6)
> ++wordlength[6];

As has been pointed out, you can write ++wordlength[nchar] (provided you
test that nchar is not too big).

> if (state == OUT)
> nchar = 0;
> }
> for (i = 50; i >= 0; --i)
> if (wordlength[1] == verscale[i])
> for (i = verscale[i]; i >= 0; --i)
> printf("%2d| %c\n", verscale[i], x);
> printf(" +------------------------------------\n");
>
> for (i = 0; i < 13; ++i)
> printf("%3d", horscale[i]);
> printf("\n");

This is all rather confused and I can't see what the objective is. If
you say what you are trying to do here, it would help.

>
> return 0;
> }

--
Ben.

Re: need some help with this histogram of words program........
 

On Feb 2, 10:01*am, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
> Ceriousmall <divadsm...@gmail.com> writes:
> > This is one of the programs from the K&R Ansi C Book that you have to
> > write I've had a go at it but got kinda stumped here. So here's my
> > effort so far...... And I'm aware there is a new ISO standard *for C
> > C99 as of *March 2000 and I'm getting around to learning and adopting
> > that as well but my *main concern is actually learning the
> > language....
>
> > /* copyright 2011 Ceriousmall. . . .
>
> > * *Program prints a histogram of the length of words in its input */
>
> > #include <stdio.h>
>
> > #define IN *1 * * */* inside a word */
> > #define OUT 0 * * */* outside a word */
>
> > main()
> > {
> > * *char x;
> > * *int c, i, n, nchar, state;
> > * * * * int wordlength[13];
> > * *int horscale[13], verscale[51];
>
> These "magic numbers" should be given names with a #define or an enum.
>
> > * *x = '*';
> > * *state = OUT;
>
> When there are only two states I refer to avoid neutral names like
> 'state' in favour of boolean variables (in C90 these are just int
> variables) with a biased name: 'in_a_word' or 'between_words' and so
> on. *That way, you don't need to name the states.
>
> > * *n = nchar = 0;
>
> > * *for (i = 0; i < 13; ++i) {
> > * * * * * *wordlength[i] = 0;
> > * * * * * *horscale[i] = i;
> > * *}
> > * *for (i = 0; i < 51; ++i)
> > * * * * * *verscale[i] = i;
>
> You set verscale[i] to i and never change it. *That's not a useful
> array -- you can always just use the index instead; but then you also
> don't use horscale in any useful way. *Without any idea what these
> array are there for I can't suggest what you should be doing with them.
> You don't need either for a simple histogram with horizontal bars.
>
> > * *while ((c=getchar()) != EOF) {
> > * * * * * *if (c == ' ' || c == '\n' || c == '\t')
> > * * * * * * * * * *state = OUT;
> > * * * * * *else if (state == OUT) * /* condition controlls the start */
> > * * * * * * * * * *state = IN; * * */* of a word */
>
> Look at the last if a bit more. *What happens when state is IN? *The
> effect will be to leave state as IN so you could just have written
>
> * *if (c == ' ' || c == '\n' || c == '\t')
> * * * * state = OUT;
> * *else state = IN;
>
> or even
>
> * state = (c == ' ' || c == '\n' || c == '\t') ? OUT : IN;
>
> If you take my idea of a boolean rather than a neutral state variable,
> you could write:
>
> * between_words = (c == ' ' || c == '\n' || c == '\t');
>
>
>
> > * * * * * *if (state == IN)
> > * * * * * * * * * *++nchar;
> > * * * * * *else if (nchar == 1)
> > * * * * * * * * * *++wordlength[1];
> > * * * * * *else if (nchar == 2)
> > * * * * * * * * * *++wordlength[2];
> > * * * * * *else if (nchar == 3)
> > * * * * * * * * * *++wordlength[3];
> > * * * * * *else if (nchar == 4)
> > * * * * * * * * * *++wordlength[4];
> > * * * * * *else if (nchar == 5)
> > * * * * * * * * * *++wordlength[5];
> > * * * * * *else if (nchar == 6)
> > * * * * * * * * * *++wordlength[6];
>
> As has been pointed out, you can write ++wordlength[nchar] (provided you
> test that nchar is not too big).
>
> > * * * * * *if (state == OUT)
> > * * * * * * * * * *nchar = 0;
> > * *}
> > * *for (i = 50; i >= 0; --i)
> > * * * * * *if (wordlength[1] == verscale[i])
> > * * * * * * * * * *for (i = verscale[i]; i >= 0; --i)
> > * * * * * * * * * * * * * *printf("%2d| *%c\n", verscale[i], x);
> > * *printf(" *+------------------------------------\n");
>
> > * *for (i = 0; i < 13; ++i)
> > * * * * * *printf("%3d", horscale[i]);
> > * *printf("\n");
>
> This is all rather confused and I can't see what the objective is. *If
> you say what you are trying to do here, it would help.
>
>
>
> > * *return 0;
> > }
>
> --
> Ben.



Ok guys I'm going to give it another go. You all have been extremely
helpful and ill post an update when I'm done. It's amazing how
powerful different ideas can be..............

Re: need some help with this histogram of words program........
 

On Feb 2, 6:01*am, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
> Ceriousmall <divadsm...@gmail.com> writes:
> > This is one of the programs from the K&R Ansi C Book that you have to
> > write I've had a go at it but got kinda stumped here. So here's my
> > effort so far...... And I'm aware there is a new ISO standard *for C
> > C99 as of *March 2000 and I'm getting around to learning and adopting
> > that as well but my *main concern is actually learning the
> > language....
>
> > /* copyright 2011 Ceriousmall. . . .
>
> > * *Program prints a histogram of the length of words in its input */
>
> > #include <stdio.h>
>
> > #define IN *1 * * */* inside a word */
> > #define OUT 0 * * */* outside a word */
>
> > main()
> > {
> > * *char x;
> > * *int c, i, n, nchar, state;
> > * * * * int wordlength[13];
> > * *int horscale[13], verscale[51];
>
> These "magic numbers" should be given names with a #define or an enum.
>
> > * *x = '*';
> > * *state = OUT;
>
> When there are only two states I refer to avoid neutral names like
> 'state' in favour of boolean variables (in C90 these are just int
> variables) with a biased name: 'in_a_word' or 'between_words' and so
> on. *That way, you don't need to name the states.
>
> > * *n = nchar = 0;
>
> > * *for (i = 0; i < 13; ++i) {
> > * * * * * *wordlength[i] = 0;
> > * * * * * *horscale[i] = i;
> > * *}
> > * *for (i = 0; i < 51; ++i)
> > * * * * * *verscale[i] = i;
>
> You set verscale[i] to i and never change it. *That's not a useful
> array -- you can always just use the index instead; but then you also
> don't use horscale in any useful way. *Without any idea what these
> array are there for I can't suggest what you should be doing with them.
> You don't need either for a simple histogram with horizontal bars.
>
> > * *while ((c=getchar()) != EOF) {
> > * * * * * *if (c == ' ' || c == '\n' || c == '\t')
> > * * * * * * * * * *state = OUT;
> > * * * * * *else if (state == OUT) * /* condition controlls the start */
> > * * * * * * * * * *state = IN; * * */* of a word */
>
> Look at the last if a bit more. *What happens when state is IN? *The
> effect will be to leave state as IN so you could just have written
>
> * *if (c == ' ' || c == '\n' || c == '\t')
> * * * * state = OUT;
> * *else state = IN;
>

I don't get what happens in the OP code if the state is IN.

Chad

Re: need some help with this histogram of words program........
 

Chad <cdalten@gmail.com> writes:

> On Feb 2, 6:01Â*am, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
>> Ceriousmall <divadsm...@gmail.com> writes:
<snip>
>> > Â* Â* Â* Â* Â* Â*if (c == ' ' || c == '\n' || c == '\t')
>> > Â* Â* Â* Â* Â* Â* Â* Â* Â* Â*state = OUT;
>> > Â* Â* Â* Â* Â* Â*else if (state == OUT) Â* /* condition controlls the start */
>> > Â* Â* Â* Â* Â* Â* Â* Â* Â* Â*state = IN; Â* Â* Â*/* of a word */
>>
>> Look at the last if a bit more. Â*What happens when state is IN? Â*The
>> effect will be to leave state as IN so you could just have written
>>
>> Â* Â*if (c == ' ' || c == '\n' || c == '\t')
>> Â* Â* Â* Â* state = OUT;
>> Â* Â*else state = IN;
>
> I don't get what happens in the OP code if the state is IN.

It might help if you said what the problem is -- the code does not seem
very complex to me.

When 'state' is 'IN', state will either be changed to 'OUT' (if 'c' is
one of three particular characters) or it will be left alone and remain
'IN'.

--
Ben.

Re: need some help with this histogram of words program........
 

On Feb 3, 10:20*pm, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
> Chad <cdal...@gmail.com> writes:
> > On Feb 2, 6:01*am, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:
> >> Ceriousmall <divadsm...@gmail.com> writes:
> <snip>
> >> > * * * * * *if (c == ' ' || c == '\n' || c == '\t')
> >> > * * * * * * * * * *state = OUT;
> >> > * * * * * *else if (state == OUT) * /* condition controlls the start */
> >> > * * * * * * * * * *state = IN; * * */* of a word */
>
> >> Look at the last if a bit more. *What happens when state is IN? *The
> >> effect will be to leave state as IN so you could just have written
>
> >> * *if (c == ' ' || c == '\n' || c == '\t')
> >> * * * * state = OUT;
> >> * *else state = IN;
>
> > I don't get what happens in the OP code if the state is IN.
>
> It might help if you said what the problem is -- the code does not seem
> very complex to me.
>
> When 'state' is 'IN', state will either be changed to 'OUT' (if 'c' is
> one of three particular characters) or it will be left alone and remain
> 'IN'.
>
> --
> Ben.

================================================== ================================================== ===================================
this group has been extremely helpful to me so thanks for all the
replies and after loads of effort this is my final product please
comment and let me know what you guys think....

/* copyright 2011 Ceriousmall. . . .

Program prints a histogram of the length of words in its input */

#include <stdio.h>

#define ONE 1 /* set to the value of one */
#define ZERO 0 /* set to nil value */
#define LIMIT 14 /* horizontal scale & word size limits */
#define VERLIMIT 51 /* verticle scale limit */

main()
{
int i, x, in_word, mark_scale;
long c, nchar;
int mark[LIMIT];
int horscale[LIMIT-ONE], verscale[VERLIMIT];
long wordlength[LIMIT];

nchar = mark_scale = ZERO;

for (i = ZERO; i < LIMIT; ++i) {
wordlength[i] = ZERO;
mark[i] = ZERO;
}
for (i = ZERO; i < LIMIT-ONE; ++i)
horscale[i] = i;

for (i = ZERO; i < VERLIMIT; ++i)
verscale[i] = i;

while ((c=getchar()) != EOF) {
if (c == ' ' || c == '\n' || c == '\t')
in_word = ZERO;
else
in_word = ONE;

if (in_word == ONE)
++nchar;
else if (nchar < LIMIT) {
for (i = ONE; i < LIMIT; ++i)
if (nchar == i)
++wordlength[i];
}
else
++wordlength[LIMIT-ONE];

if (in_word == ZERO)
nchar = ZERO;
}
for (i = VERLIMIT-ONE; i > ZERO; --i) {
for (x = ONE; x < LIMIT; ++x)
if (wordlength[x] == verscale[i])
++mark_scale;

if (mark_scale > ZERO)
printf("%2d|", verscale[i]);

for (x = ONE; x < LIMIT; ++x)
if (wordlength[x] == verscale[i]) {
printf(" %c", '*');
++mark[x];
}
else if (mark[x] == ZERO)
printf(" %c", ' ');
else
printf(" %c", '*');
printf("\n");
}
printf(" +---------------------------------------\n");

for (i = ZERO; i < LIMIT-ONE; ++i)
printf("%3d", horscale[i]);

printf(" >12\n");

return 0;
}
_______________________________________________
I'm really beginning to C the code now. . . . .

Re: need some help with this histogram of words program........
 

I think this is better tho...................

/* copyright 2011 Ceriousmall. . . .

Program prints a histogram of the length of words in its input */

#include <stdio.h>

#define ONE 1 /* set to the value of one */
#define ZERO 0 /* set to nil value */
#define LIMIT 14 /* horizontal scale & word size limits */
#define VERLIMIT 51 /* verticle scale limit */

main()
{
int i, x, in_word, mark_scale;
long c, nchar;
int mark[LIMIT];
int horscale[LIMIT-1], verscale[VERLIMIT];
long wordlength[LIMIT];

nchar = mark_scale = 0;

for (i = 0; i < LIMIT; ++i) {
wordlength[i] = 0;
mark[i] = 0;
}
for (i = 0; i < LIMIT-1; ++i)
horscale[i] = i;

for (i = 0; i < VERLIMIT; ++i)
verscale[i] = i;

while ((c=getchar()) != EOF) {
if (c == ' ' || c == '\n' || c == '\t')
in_word = ZERO;
else
in_word = ONE;

if (in_word == ONE)
++nchar;
else if (nchar < LIMIT) {
for (i = 1; i < LIMIT; ++i)
if (nchar == i)
++wordlength[i];
}
else
++wordlength[LIMIT-1];

if (in_word == ZERO)
nchar = ZERO;
}
for (i = VERLIMIT-1; i > 0; --i) {
for (x = 1; x < LIMIT; ++x)
if (wordlength[x] == verscale[i])
++mark_scale;

if (mark_scale > 0)
printf("%2d|", verscale[i]);

for (x = 0; x < LIMIT; ++x)
if (wordlength[x] == verscale[i]) {
printf(" %c", '*');
++mark[x];
}
else if (mark[x] == 0)
printf(" %c", ' ');
else
printf(" %c", '*');
printf("\n");
}
printf(" +---------------------------------------\n");

for (i = 0; i < LIMIT-1; ++i)
printf("%3d", horscale[i]);

printf(" >12\n");

return 0;
}

Re: need some help with this histogram of words program........
 

Ceriousmall <divadsmall@gmail.com> writes:

> I think this is better tho...................
>
> /* copyright 2011 Ceriousmall. . . .
>
> Program prints a histogram of the length of words in its input */
>
> #include <stdio.h>
>
> #define ONE 1 /* set to the value of one */
> #define ZERO 0 /* set to nil value */

I don't think these two comments really help.

What's more, I don't think these macros are a good idea. By all means
write something like

#define TRUE 1
#define FALSE 0

or

enum { false = 0, true = 1 };

if you don't like writing 'in_word = 0;' but naming 0 'ZERO' and 1 'ONE'
is not helpful. Consider the difference between

#define LIMIT 14

which is fine (though I might prefer a more descriptive name) and

#define FOURTEEN 14

> #define LIMIT 14 /* horizontal scale & word size limits */
> #define VERLIMIT 51 /* verticle scale limit */
>
> main()

This should be 'int main(void)'.

> {
> int i, x, in_word, mark_scale;
> long c, nchar;

Why are these two declared long? There's nothing wrong with that, it's
just a bit odd and will puzzle anyone reading the code.

> int mark[LIMIT];
> int horscale[LIMIT-1], verscale[VERLIMIT];
> long wordlength[LIMIT];
>
> nchar = mark_scale = 0;
>
> for (i = 0; i < LIMIT; ++i) {
> wordlength[i] = 0;
> mark[i] = 0;
> }

You can just initialise the arrays if you like:

long wordlength[LIMIT] = {0};
int mark[LIMIT] = {0};

> for (i = 0; i < LIMIT-1; ++i)
> horscale[i] = i;
>
> for (i = 0; i < VERLIMIT; ++i)
> verscale[i] = i;

These two are still not used. Yes, they are reference below but they
serve no purpose.

> while ((c=getchar()) != EOF) {
> if (c == ' ' || c == '\n' || c == '\t')
> in_word = ZERO;
> else
> in_word = ONE;

I won't say what I'd write here -- you'd be appalled!

> if (in_word == ONE)
> ++nchar;
> else if (nchar < LIMIT) {
> for (i = 1; i < LIMIT; ++i)
> if (nchar == i)
> ++wordlength[i];

Why have a loop here? You don't need to loop just to catch the case when
nchar == i to increment wordlength[i]. You can increment
wordlength[nchar] directly (though the loop does catch the case when
nchar is zero).

> }
> else
> ++wordlength[LIMIT-1];
>
> if (in_word == ZERO)
> nchar = ZERO;
> }

Look at the whole body of the while loop. You test for a space-like
character and set 'in_word' then you immediately test it and do one
thing if it is true (++nchar) and another thing if it is false
(increment an element of wordlength and set nchar to zero).

Can you see how you could write it much more simply?

> for (i = VERLIMIT-1; i > 0; --i) {
> for (x = 1; x < LIMIT; ++x)
> if (wordlength[x] == verscale[i])
> ++mark_scale;
>
> if (mark_scale > 0)
> printf("%2d|", verscale[i]);

This confused me for a bit. Incrementing mark_scale and testing for > 0
suggests and arithmetic purpose, but really it is a Boolean (true/false)
value just like 'in_word' above.

> for (x = 0; x < LIMIT; ++x)
> if (wordlength[x] == verscale[i]) {
> printf(" %c", '*');
> ++mark[x];
> }
> else if (mark[x] == 0)
> printf(" %c", ' ');
> else
> printf(" %c", '*');
> printf("\n");
> }

You can remove 'verscale' simply by writing 'i' instead of
'verscale[i]'. You can also get rid of the 'mark' array. In the inner
loop, the condition that controls whether a star or a space is written
is much simpler than you have made it. It took me a while to work out
what the 'mark' array is being used for because it is really a Boolean
(true/false) array yet you increment it's elements (just as with
'mark_scale' above).

You code simply ignores counts that are too big. That's not a good
idea. The options would seem to be (a) scale the histogram so that
highest bar is always VERLIMIT lines high; (b) just print a very tall
histogram; or (c) limit the height but show that some bars are higher
than actually shown.

> printf(" +---------------------------------------\n");
>
> for (i = 0; i < LIMIT-1; ++i)
> printf("%3d", horscale[i]);

horscale[i] == i so there is no need for this array.

There are also a few counting bugs. For example, input that has just
one one-word line produces:

1| *
+---------------------------------------
0 1 2 3 4 5 6 7 8 9 10 11 12 >12

> printf(" >12\n");

12? LIMIT is 14.

> return 0;
> }

--
Ben.

Re: need some help with this histogram of words program........
 

On Feb 6, 7:05*pm, Ben Bacarisse <ben.use...@bsb.me.uk> wrote:

Hey thanks for the pointers Ben I've certainly missed a lot. As simple
as you make it seem, trust me it wasn't so for me but this is getting
easier. These reviews are extremely useful to me as there isn't a
whole lot of people I know that code in C...... Well I'm off to Vi
again well Elvis that is............





_______________________________________________
Ceriously
I'm really beginning to C the code now. . . . .