Print the length of words as a vertical histogram

Question

I thought I'd give it a shot. What I came up with is from a culmination of my last two submissions, incorporating most of the feedback from this community, including better naming of variables, initialization, and making my program a bit more user-friendly. I'm not sure which optimizations I can make or if there are any issues with my code.

Any feedback is welcome!

#include <stdio.h>

/* print the length of words as input to a histogram with vertical bars */
int main(void) {
  int c, i, j;
  int word_length = 0;
  int max_word_frequency = 0;
  int histogram[10] = {0};

  putchar('>');
  while ((c = getchar()) != EOF && word_length < 10) {
    if (c != ' ' && c != '\n' && c != '\t') {
      ++word_length;
    }
    else {
      ++histogram[word_length];
      word_length = 0;
    }
  }
  for (i = 0; i < 10; ++i) {
    printf("%d ", i);
    if (histogram[i] > max_word_frequency)
      max_word_frequency = histogram[i];
  }
  putchar('\n');
  for (i = 0; i < max_word_frequency; ++i) {
    for (j = 0; j < 9; ++j) {
      if (histogram[j] > 0) {
         printf("x ");
         histogram[j] = histogram[j] -1;
      }
      else {
         printf("  ");
      }
    }
  putchar('\n');
  }
}

Answer 1

main()

Your main() does three things: print a prompt, count the frequencies, and print the histogram. The latter two, in particular, are well-defined complex tasks, they each deserve to be a function.

If the input is redirected from a file, rather than typed directly on the console, then the > prompt just messes up the output. To see if standard input is a console, you should test isatty() on Unix, or _isatty() on Windows.

int main() {
    int histogram[21];
    if (isatty(fileno(stdin))) {
        putchar('>');
    }
    count_word_lengths(stdin, histogram, sizeof histogram / sizeof histogram[0]);
    print_histogram(histogram, sizeof histogram / sizeof histogram[0]);
}

Counting

If the input ends abruptly with a word followed by EOF, with no intervening whitespace, then the last word doesn't get counted.

Instead of if (c != ' ' && c != '\n' && c != '\t'), consider using tests like isspace() and ispunct(). By the way, you are treating punctuation as if they were part of words, which is probably not the right behaviour.

If the input contains any word with length 10 or greater, then the counting terminates prematurely with no indication that anything went wrong. (Personally, I'd choose to count all excessively long words as if it were the maximum length supported by the histogram array.)

Printing

Destroying the histogram data as you print is a bad idea, especially if it will cause a print_histogram() function to wipe out its input parameter.

Once you fix that, the printf("x ") and printf(" ") would be better with a ternary expression.

for (int row = 1; row <= max_word_frequency; ++row) {
    for (int i = 0; i < 9; ++i) {
        printf(row <= histogram[i] ? "x " : "  ");
    }
    putchar('\n');
}

Answer 2

Rather the use 10 throughout the code, define a constant in one place.
Certainly makes code easier to update to a new size and self-documents the "meaning" of 10

#define HISTOGRAM_WIDTH 10
int histogram[HISTOGRAM_WIDTH] = {0};
...
while ((c = getchar()) != EOF && word_length < HISTOGRAM_WIDTH) {
  ...
  }
for (i = 0; i < HISTOGRAM_WIDTH; ++i) {
  ...
for (j = 0; j < HISTOGRAM_WIDTH - 1; ++j) {
  ...

BTW: This makes the j < HISTOGRAM_WIDTH - 1 suspicious. Should code have been j < HISTOGRAM_WIDTH?