K&R 1-14 - Histogram

Question

This is the K&R Exercise 1-14 of C. The exercise ask you to print a histogram of the frequencies of different characters in input. How can I improve my code? The main doubt is the part when I have to increase letters in the array.

#include <stdio.h>
#include <ctype.h>

int main()
{
    int ch, 
        nwhite, 
        nother;

    int num[10] = {0};
    int lett[26] = {0};

    nwhite = nother = 0;

    while ((ch = getchar()) != EOF)
    {
        if (isdigit(ch))
            ++num[ch-'0'];
        else if (isalpha(ch))
        {
            if(ch >='A' && ch <= 'Z')
                ch = tolower(ch);

            if(ch >= 'a' && ch <= 'k')
                ++lett[ch-'a'];
            else if(ch >= 'l' && ch <= 'v')
                ++lett[ch-'l'];
            else if(ch >= 'w' && ch <= 'z')
                ++lett[ch-'w'];
        }

        else if (ch == ' ' || ch == '\n' || ch == '\t')
            ++nwhite;
        else
            ++nother;
    }

    putchar('\n');

    printf("Numbers|");
    for(int i = 0; i <= 10; ++i)
    {
        for(int j = 0; j < num[i]; ++j)
            putchar('*');
    }
    putchar('\n');

    printf("Letters|");
    for(int i = 0; i <= 10; ++i)
    {
        for(int j = 0; j < lett[i]; ++j)
            putchar('*');
    }
    putchar('\n');

    printf("White spaces|");
    for(int i = 0; i <= nwhite; ++i)
    {
        if(nwhite - i > 0)
            putchar('*');
    }
    putchar ('\n');

    printf("Others|");
    for(int i = 0; i <= nother; ++i)
    {
        if(nother - i > 0)
            putchar('*');
    }
    putchar ('\n');

    return 0;
}

Answer 1

• Bugs.

  • The loop printing the histogram lines for numbers,

    for(int i = 0; i <= 10; ++i)
    

  lets i become 10, which accesses num[10]. It does not exist. Undefined behavior it is.

  A c idiom is to use < in a loop termination condition. There are cases which require <=, but they are very rare.

  • The loop printing the histogram lines for letters,

    for(int i = 0; i <= 10; ++i)
    

  only reports counters for a to k inclusive. All other letters are not reported.

• The second bug tells why magic numbers are bad. You want to report all counters in the lett array. There are sizeof(lett) / sizeof(lett[0]) of them:

      for(int i = 0; i < sizeof(lett) / sizeof(lett[0]); ++i)
  

  Ditto for num.

• It is very unclear why you split handling letters into three cases. A simple

      else if (isalpha(ch)) {
          ch = tolower(ch); // No need to test for `isupper`
          ++lett[ch - `a`];
      }
  

  is enough.

• There are more whitespaces than ' ', '\n', '\t'. The ctypes.h has isspace function. Use it.

• Testing for nwhite - i > 0 is yet another manifestation of why <= loop condition is wrong.

      for(int i = 0; i < nwhite; ++i) {
          putchar('*');
      }
  

  is all you need.

• Once you fix the last two loops, all the loops doing an actual printing become identical. Factor them out into a function

  void print_stars(int n)
  {
      for (int i = 0; i < n; i++) {
          putchar('*');
      }
      putchar('\n');
  }