K&R2 1.6 Arrays, exercise 1-13 (vertical histogram)

[arnuld]

i was able to create a solution for a Horizontal-Histogram. i was
completely unable to understand this Vertical-Histogram phenomenon.
even though i have looked at the solution at this page:

http://clc-wiki.net/wiki/K&R2_solutions:Chapter_1:Exercise_13

still i am not able to make sense of many-many things like (on the
same page) for my solution i have written this in documentation,to
give reader some hint about what i thought before coding:


METHOD:


1.) we will count the length of each word by counting the
characters it has.

2.) we use an array where we will keep track of how many words
of a specific length we have encountered.

3.) for simplicity, we will not take into account any words
having more than 10 characters in it. extra characters will
simply be discarded.

*/

and this:

int i = 0;
int j =0; /* i and j are simply index counters */
int c = 0;
int nchar = 0; /* number of characters in a word */
int inspace = IN; /* a flag to know whether we are inside of outside
the word */
int wordlen[MAXLENGTH]; /* counts how many words of a particular
length we have seen */


but the Vertical-Histogram solution doe snot given any hints in
variables. completely alien kind of thing for me. does that mean i
should leave programming and do something else like carpentry or
selling water purifiers :-(

 

[Flash Gordon]

arnuld wrote, On 04/04/07 19:03:

i was able to create a solution for a Horizontal-Histogram. i was
completely unable to understand this Vertical-Histogram phenomenon.
even though i have looked at the solution at this page:

http://clc-wiki.net/wiki/K&R2_solutions:Chapter_1:Exercise_13

still i am not able to make sense of many-many things like (on the
same page)

You should ask specific questions then.

> for my solution i have written this in documentation,to
give reader some hint about what i thought before coding:

Yes, providing documentation is helpful.

and this:

int i = 0;
int j =0; /* i and j are simply index counters */

The above comment is pointless since by convention i and j are used as
counters.

but the Vertical-Histogram solution doe snot given any hints in
variables.

On looking at it I found the variable names to be self explanatory. I
also have a couple of other comments on Richard's and your solutions
which I will come to.

> completely alien kind of thing for me. does that mean i
should leave programming and do something else like carpentry or
selling water purifiers :-(

It takes time to learn. Richard has a good few years of experience. I'm
guessing that you are 20 or under, if I am right then I started
programming before you were born. You just have to work at it.

Comments on Richard's code...
| int main(void)
| {
| int c;

I thought you always initialised variables on declaration, or am I
thinking of someone else ;-)

int inspace = 0;
long lengtharr[MAXWORDLEN + 1];

for(thisidx = 0; thisidx <= MAXWORDLEN; thisidx++)
{
lengtharr[thisidx] = 0;
}

This, of course, could have been done by initialising on declaration as
you know, I assume you did not because that has not been covered at that
point in K&R?

while(done == 0)

If you had initialised c to something other than EOF you could have used:
while (c!=EOF)
thus saving yourself an extra state variable.

{
c = getchar();

If you do not do what I suggest above, then get rid of the earlier
declaration of c and declare it here with
int c = getchar();
Thus reverting to what I believe is your normal form

if(c == ' ' || c == '\t' || c == '\n' || c == EOF)
{

Now for comments on Arnuld's code:
| #define IN 1
| #define OUT 0

I would use an enum or just rely on the normal C convention of 0 being
false and non-zero being true.

| int main()

int main(void) is better as a matter of habit.

| int c = EOF;
| int i = 0;
| int j = 0;

I certainly would not bother initialising i or j since they are used as
simple loop counters and initialised in the loop. I would not bother
initialising c either.

| int arr_of_len[MAXWORDLEN + 1];
| int state = IN;
| int nc = 0;

<snip>

| while((c = getchar()) != EOF)
| {

Read the comments at the top of Richard's code. As with his original
solution (not the one on the Wiki) you will not count the last word if
the last line does not end in a space, tab or newline.

| ++nchar;
| if(c == '"' || c == '\'')
| --nchar;

Why not make this simpler and use
if(c != '"' && c != '\'')
++nchar;

 

[user923005]

Why not think up your own solution to K&R exercise 1-13?
Have you covered arrays yet?

 

[Richard Heathfield]

Flash Gordon said:

Comments on Richard's code...
| int main(void)
| {
| int c;

I thought you always initialised variables on declaration, or am I
thinking of someone else ;-)

Firstly, I know you're just yanking my chain. But you want to hear it
rattle, so I'll keep going.

Secondly, you're looking at code that is almost a decade old.

Thirdly, my decision to adopt blanket initialisation was made after many
years of *not* doing it, and even nowadays I find that old habits die
hard. But, if it helps, I regard the above code as containing a bug.

int inspace = 0;
long lengtharr[MAXWORDLEN + 1];

for(thisidx = 0; thisidx <= MAXWORDLEN; thisidx++)
{
lengtharr[thisidx] = 0;
}

This, of course, could have been done by initialising on declaration
as you know, I assume you did not because that has not been covered at
that point in K&R?

Either that or because, when I first wrote the code (which was many
years ago), it's entirely possible that I *didn't* know the = {0}
thing.

(If the latter reason is true, learning it would have done no good here,
because Reason 1 would still have applied.)

If you had initialised c to something other than EOF you could have
used:
while (c!=EOF)
thus saving yourself an extra state variable.

Yes, I could have done that. Some people may even consider it a good
idea. In my view, however, the state variable makes it easier for a
newbie to read the code.

 

[arnuld]

Why not think up your own solution to K&R exercise 1-13?

this is all i can do:

#include<stdio.h>

int main()
{
return 0;
}

Have you covered arrays yet?

not sure what you mean. i am at chapter 1, so i know only this:

type arr[size];

"arr" is an array of type "type" and "size" tells the number of
elements it has. that's it.

 

[arnuld]

You should ask specific questions then.

ok, i will try but there are lost of questions then.

Yes, providing documentation is helpful.

thanks. i made that point because once i read some C++ code written by
a teacher who teaches C++ and C , 200 lines long, and it was a
complete mess with 61 errors, after that i decided to use lots of /*
and */ in my code

The above comment is pointless since by convention i and j are used as
counters.

yes, i felt that after some time. i am a newbie.

It takes time to learn. Richard has a good few years of experience. I'm
guessing that you are 20 or under, if I am right then I started
programming before you were born. You just have to work at it.

i am 26, started programming 1 year ago but i think i need to work at
it.

for(thisidx = 0; thisidx <= MAXWORDLEN; thisidx++)
{
lengtharr[thisidx] = 0;
}

This, of course, could have been done by initialising on declaration as
you know, I assume you did not because that has not been covered at that
point in K&R?

i wanted to ask. what exactly "thisidx" is here for. "this identity x"
or simply "this ID x" but it refers to which "identity" ?

Now for comments on Arnuld's code:
| #define IN 1
| #define OUT 0

I would use an enum or just rely on the normal C convention of 0 being
false and non-zero being true.

chapter 1, i have not encountered ENUM yet.

| int main()

int main(void) is better as a matter of habit.

according to C99 ?

i compile my code with this:

gcc -ansi -pedantic -Wall -Wextra -O input.c

but if you say "-std=c99" is a better idea then i will do that.

| int c = EOF;
| int i = 0;
| int j = 0;

I certainly would not bother initialising i or j since they are used as
simple loop counters and initialised in the loop. I would not bother
initialising c either.

actually, i get that habit from Richard Heathfiled. he made some point
regarding uninitialised variables.

| ++nchar;
| if(c == '"' || c == '\'')
| --nchar;

Why not make this simpler and use
if(c != '"' && c != '\'')
++nchar;

good, it took me a while before i understood it. what about these at
http://clc-wiki.net/wiki/K&R2_solutions:Chapter_1:Exercise_13 :

if(inspace == 0)
{
firstletter = 0;
inspace = 1;

what does that mean ? we are neither inside a space nor at the first
letter of a word.

if(c == EOF)
{
done = 1;
}

it could have been at the beginning of "main".


if(inspace == 1 || firstletter == 1)
{
wordlen = 0;
firstletter = 0;
inspace = 0;

now we are inside a space, means we are at a space where previous
character was also a space, and we are also at the first letter of a
word and then we are setting "word length = 0"

??

 

[arnuld]

Flash Gordon said:


Yes, I could have done that. Some people may even consider it a good
idea. In my view, however, the state variable makes it easier for a
newbie to read the code.

No it does not. to a newbie like me it makes thing difficult BUT may
be i am biased as i use K&R2 to learn C where at every place he uses:

while((c = getchar()) != EOF)

so i had a terrible understanding the significance of d == 0 or d == 1

 

[Flash Gordon]

arnuld wrote, On 05/04/07 10:55:

yes, i felt that after some time. i am a newbie.

Feel free to improve the solutions you have posted on the Wiki.

i am 26, started programming 1 year ago but i think i need to work at
it.

OK, then I had started programming before you were born, although it was
just a hobby back then not a job. I've no idea now hoe good or bad I was
after 1 year.

for(thisidx = 0; thisidx <= MAXWORDLEN; thisidx++)
{
lengtharr[thisidx] = 0;
}

This, of course, could have been done by initialising on declaration as
you know, I assume you did not because that has not been covered at that
point in K&R?

i wanted to ask. what exactly "thisidx" is here for. "this identity x"
or simply "this ID x" but it refers to which "identity" ?

I read it as "this index". This implies to me that it is just a variable
being used to index in to an array, i.e. just the same as people often
use i and j.

chapter 1, i have not encountered ENUM yet.

Just tuck the information away for future reference. Although my second
suggestion is still valid in my opinion.

according to C99 ?

Both "int main()" and "int main(void)" are valid in C89 and C99. The
difference is the specifying void as the parameter list ensures that if
you call main with any parameters after the declaration the compiler
will complain. This is more important for other function declarations,
since in general you do not call main (although it is legal), so it is a
good habit rather than necessarily that important in this specific instance.

actually, i get that habit from Richard Heathfiled. he made some point
regarding uninitialised variables.

Not everyone agrees with Richard on everything. I agree that it is a
good habit, but it is something I am less strict on than Richard.

good, it took me a while before i understood it. what about these at
http://clc-wiki.net/wiki/K&R2_solutions:Chapter_1:Exercise_13 :

if(inspace == 0)
{
firstletter = 0;
inspace = 1;

what does that mean ? we are neither inside a space nor at the first
letter of a word.

IF we were not in a space
THEN
this is not the first letter
we are now inside a space

is how I would read that.

if(c == EOF)
{
done = 1;
}

it could have been at the beginning of "main".

It could have been, but the above is after an attempt has been made to
read a character in to c.

if(inspace == 1 || firstletter == 1)
{
wordlen = 0;
firstletter = 0;
inspace = 0;

now we are inside a space, means we are at a space where previous
character was also a space, and we are also at the first letter of a
word and then we are setting "word length = 0"

??

IF we are currently in a space OR this is the first letter in the file
THEN
set the word length to 0
after this we are not dealing with the first letter
we are not in a space

I would probably do some of this slightly differently.

I have never done the K&R exercises because of when and how I learned C,
although I was reading and referring to K&R2 as part of my learning.

 

[Flash Gordon]

Richard Heathfield wrote, On 05/04/07 01:47:

Flash Gordon said:



Firstly, I know you're just yanking my chain. But you want to hear it
rattle, so I'll keep going.

Indeed I was.

Secondly, you're looking at code that is almost a decade old.

I did not register that it was that old.

Thirdly, my decision to adopt blanket initialisation was made after many
years of *not* doing it, and even nowadays I find that old habits die
hard. But, if it helps, I regard the above code as containing a bug.

I would not consider it a bug as such, just less than optimal. You can,
of course, improve your code on the Wiki.

int inspace = 0;
long lengtharr[MAXWORDLEN + 1];

for(thisidx = 0; thisidx <= MAXWORDLEN; thisidx++)
{
lengtharr[thisidx] = 0;
}

This, of course, could have been done by initialising on declaration
as you know, I assume you did not because that has not been covered at
that point in K&R?

Either that or because, when I first wrote the code (which was many
years ago), it's entirely possible that I *didn't* know the = {0}
thing.

(If the latter reason is true, learning it would have done no good here,
because Reason 1 would still have applied.)

Yes, I agree.

Yes, I could have done that. Some people may even consider it a good
idea. In my view, however, the state variable makes it easier for a
newbie to read the code.

Readability would be a valid reason whether it was for the newbie
audience or for serious code to be maintained by highly skilled
professionals. I would not have bothered with the comment if I was
reviewing the code professionally, I only commented on it at all because
of your comment above the code about adding the extra state variable.

In a review I would have been more likely to suggest replacing the while
loop with a do-while loop since it is explicitly a "do at lest once" loop.

Of course, there is also the option of using a break instead of a state
variable. However, I would not consider this to be an improvement,
merely a change which some have valid reasons to dislike.

The most serious complaints I have on your code are that you are not
checking for overflow on your counts, passing a long to printf having
told it to expect an int!
| long thisval = 0;
<snip>
| printf("%4d | ", thisval);


Your output will also be messed up if you have too many of a given word
length.

Now, if Arnuld has followed this discussion of your code (my main reason
for commenting on it was so that he could follow the discussion) he
should have learnt a couple of additional things.

 

[Flash Gordon]

Flash Gordon wrote, On 05/04/07 18:57:

Richard Heathfield wrote, On 05/04/07 01:47:

Indeed I was.


I did not register that it was that old.

The most serious complaints I have on your code are that you are not
checking for overflow on your counts, passing a long to printf having
told it to expect an int!
| long thisval = 0;
<snip>
| printf("%4d | ", thisval);


Your output will also be messed up if you have too many of a given word
length.

I know it's bad form to follow up on oneself, but here is my improved
version of your code. This definitely uses things that someone on
chapter 1 of K&R2 would not know (although it might be of interest to
people working through K&R2). I would be interested to know if I've
missed other than a possibly dubious definition of what a word is.

#include <stdio.h>
#include <limits.h>

#define MAXWORDLEN 10

int main(void)
{
int c;
int inspace = 0;
unsigned long lengtharr[MAXWORDLEN + 1] = { 0 };
int wordlen = 0;
unsigned long thisval;
unsigned long maxval = 0;
int countdigits = 1;
unsigned long countdiglim = 10;
int thisidx;

do {
c = getchar();

if (c == ' ' || c == '\t' || c == '\n' || c == EOF) {
if (!inspace) {
inspace = 1;

if (wordlen > 0) {
if (wordlen > MAXWORDLEN+1)
wordlen = MAXWORDLEN+1;

if (lengtharr[wordlen - 1] < ULONG_MAX) {
thisval = ++lengtharr[wordlen - 1];
if (thisval > maxval) {
maxval = thisval;
if (maxval >= countdiglim) {
++countdigits;
countdiglim *= 10;
}
}
}
}
wordlen = 0;
}
}
else {
inspace = 0;
wordlen++;
}
} while (c != EOF);

for (thisval = maxval; thisval > 0; thisval--) {
if (thisval == ULONG_MAX)
printf("%*s | ", countdigits, "lots");
else
printf("%*lu | ", countdigits, thisval);
for (thisidx = 0; thisidx <= MAXWORDLEN; thisidx++) {
if (lengtharr[thisidx] >= thisval)
printf("* ");
else
printf(" ");
}
printf("\n");
}

printf(" %*s+",countdigits,"");
for(thisidx = 0; thisidx <= MAXWORDLEN; thisidx++)
printf("---");

printf("\n %*s",countdigits,"");
for(thisidx = 0; thisidx < MAXWORDLEN; thisidx++)
printf("%2d ", thisidx + 1);
printf(">%d\n", MAXWORDLEN);

return 0;
}

I have tested this against Richard's version and it seems to work.

 

[user923005]

Why not think up your own solution to K&R exercise 1-13?

this is all i can do:

#include<stdio.h>

int main()
{
return 0;

}

Have you covered arrays yet?

not sure what you mean. i am at chapter 1, so i know only this:

type arr[size];

"arr" is an array of type "type" and "size" tells the number of
elements it has. that's it.

It is just that the problem is trivial with 2-d arrays. I guess that
you did not get there yet.

 

[Richard Heathfield]

Flash Gordon said:

The most serious complaints I have on your code are that you are not
checking for overflow on your counts,
Mm-hmmm...

passing a long to printf having told it to expect an int!
WHAT?

| long thisval = 0;
<snip>
| printf("%4d | ", thisval);

Oh deary deary me. You do have a point, don't you!

Your output will also be messed up if you have too many of a given
word length.

Aye.

 

[CBFalconer]

.... snip ...

i compile my code with this:

gcc -ansi -pedantic -Wall -Wextra -O input.c

but if you say "-std=c99" is a better idea then i will do that.

In general, it is not a better idea. Most people are using C90 or
C95 compilers. K & R is written to that standard.

--
<http://www.cs.auckland.ac.nz/~pgut001/pubs/vista_cost.txt>
<http://www.securityfocus.com/columnists/423>
<http://www.aaxnet.com/editor/edit043.html>

"A man who is right every time is not likely to do very much."
-- Francis Crick, co-discover of DNA
"There is nothing more amazing than stupidity in action."
-- Thomas Matthews