K&R C requires all variables to be declared at the start of their enclosing block. This is a bad habit to learn: it's much safer to declare variables where they can be initialised:
/* these can already be initialised where they are declared */
int state = OUT;
int histo[MAXLEN+1] = { 0 }; /* C90 */
int counter = 0;
/* reduce scope, and never have uninitialized value */
for (int j = 0; j < histo[i]; ++j) /* C99 */
We could remove the need to subtract 1 from the length when updating histo if we reserve the first element for long words instead of the last:
if (state == IN && counter > 0) {
if (counter <= MAXLEN)
++histo[counter];
else
++histo[0];
}
We obviously need to make a corresponding change to the printing:
for (int i = 1; i <= MAXLEN; ++i) {
if (MAXLEN < 10)
printf(" %1d | ", i);
else if (MAXLEN < 100)
printf(" %2d |", i);
else
printf(" %3d |", i);
for (int j = 0; j < histo[i]; ++j)
putchar('*');
putchar('\n');
}
We can avoid the if/else chain on MAXLEN in this loop, by choosing the formatting string in advance:
const char *format = "";
if (MAXLEN < 10) {
format = " %1d | ";
} else if (MAXLEN < 100) {
format = " %2d | ";
} else {
format = " %3d | ";
}
for (int i = 1; i <= MAXLEN; ++i) {
printf(format, i);
for (int j = 0; j < histo[i]; ++j)
putchar('*');
putchar('\n');
}
That might not look like an improvement, but with a small change, we can use the same format string to insert > for the over-long words, too:
const char *format = "";
if (MAXLEN < 10) {
format = "%c%1d | ";
} else if (MAXLEN < 100) {
format = "%c%2d | ";
} else {
format = "%c%3d | ";
}
for (int i = 1; i <= MAXLEN; ++i) {
printf(format, ' ', i);
for (int j = 0; j < histo[i]; ++j)
putchar('*');
putchar('\n');
}
/* over-long words */
printf(format, '>', MAXLEN);
for (int j = 0; j < histo[0]; ++j)
putchar('*');
putchar('\n');
Going beyond the expected level of knowledge, we could even compute the necessary length, and pass it using %*d:
int width = 1;
for (int i = MAXLEN; i >= 10; i /= 10) {
++width;
}
for (int i = 1; i <= MAXLEN; ++i) {
printf(" %*d | ", width, i);
for (int j = 0; j < histo[i]; ++j)
putchar('*');
putchar('\n');
}
/* over-long words */
printf(">%*d | ", width, MAXLEN);
for (int j = 0; j < histo[0]; ++j)
putchar('*');
putchar('\n');
That allows us to have non-constant MAXLEN, which will be to our advantage in future when we make this a function.
Whilst the state machine is a good idea, it turns out we don't need to explicitly store state, because counter is always zero outside a word and non-zero once we're in one:
while ((c = getchar()) != EOF) {
if (c == ' ' || c == '\n' || c == '\t') {
if (counter > 0) {
++histo[counter <= MAXLEN ? counter : 0];
counter = 0;
}
} else {
++counter;
}
}
Did you spot the bug in this loop? Probably not, because the bug is missing code immediately after the loop. If the input stream ends during a word, we fail to count it. We need to repeat the if (counter > 0) test just after the loop, to catch that case.
We have an unusual definition of "word" - in our source code, strings such as ++histo[counter-1]; count as single words. Even in English text, we'd expect text, to register as a four-character word. To fix this, we could consider only alphanumerics as word characters, and that could be a good introduction to the functions in <ctype.h>.
while ((c = getchar()) != EOF) {
if (isalnum(c)) {
++counter;
} else {
if (counter > 0) {
++histo[counter <= MAXLEN ? counter : 0];
counter = 0;
}
}
}
if (counter > 0) {
++histo[counter <= MAXLEN ? counter : 0];
}
Enhancement - we could avoid bars going off the right edge of the display, by calculating a scale factor first. Here's my suggestion:
#define MAXWIDTH 72 /* screen width available for bars */
double scale = 1.0;
for (int i = 0; i <= MAXLEN; ++i) {
if (histo[i] * scale > MAXWIDTH) {
scale = 1.0 * MAXWIDTH / histo[i];
}
}
for (int j = 0; j < histo[i] * scale; ++j) { putchar('*'); }
Finally: it's not part of your C code, but there's no need for cat in the test. We can simply redirect input like this:
./ch1-ex-1-13-01 <ch1-ex-1-13-01.c
Modified code
Applying all my suggestions, I got:
/* Exercise 1-13. Write a program to print a histogram of the lengths of
words in its input. It is easy to draw the histogram with the bars
horizontal; a vertical orientation is more challenging. */
/* Solution 1: Horizontal Bars
* */
#include <ctype.h>
#include <stdio.h>
#define MAXLEN 10 /* Any word longer than this will get counted in the
>MAXLEN histogram. */
#define MAXWIDTH 72 /* screen width available for bars */
int main()
{
int histo[MAXLEN+1] = { 0 }; /* element 0 counts over-length words */
/* Perform the counting */
int counter = 0;
int c;
while ((c = getchar()) != EOF) {
if (isalnum(c)) {
++counter;
} else {
if (counter > 0) {
++histo[counter <= MAXLEN ? counter : 0];
counter = 0;
}
}
}
if (counter > 0) {
++histo[counter <= MAXLEN ? counter : 0];
}
/* Calculate sensible scale */
double scale = 1.0;
for (int i = 0; i <= MAXLEN; ++i) {
if (histo[i] * scale > MAXWIDTH) {
scale = 1.0 * MAXWIDTH / histo[i];
}
}
/* how wide are the labels? */
int width = 1;
for (int i = MAXLEN; i >= 10; i /= 10) {
++width;
}
/* Write the output */
for (int i = 1; i <= MAXLEN; ++i) {
printf(" %*d | ", width, i);
for (int j = 0; j < histo[i] * scale; ++j) { putchar('*'); }
putchar('\n');
}
/* over-long words */
printf(">%*d | ", width, MAXLEN);
for (int j = 0; j < histo[0] * scale; ++j) { putchar('*'); }
putchar('\n');
}
The second edition, "K&R2", was first published in 1988 with some updates of the book to meet the version of the language standardized by ANSI in 1989.andThe 1989 ANSI C standard was republished by ISO in 1990\$\endgroup\$ – div0man Oct 18 '18 at 16:28