Counting letters program with ASCII-art graph

I am writing a program in C running on UNIX which counts the number of each letters in a input text file. For a file like this:

The cat sat on the green mat

The output would be like this:

The letter ’a’ occurs 3 times.
   The letter ’c’ occurs 1 times.
   The letter ’e’ occurs 4 times.
   The letter ’g’ occurs 1 times.
   The letter ’h’ occurs 2 times.
   The letter ’m’ occurs 1 times.
   The letter ’n’ occurs 2 times.
   The letter ’o’ occurs 1 times.
   The letter ’r’ occurs 1 times.
   The letter ’s’ occurs 1 times.
   The letter ’t’ occurs 5 times.

  5                    *
  4     *              *
  4     *              *
  3 *   *              *
  3 *   *              *
  2 *   *  *     *     *
  2 *   *  *     *     *
  1 * * * **    ***  ***
  1 * * * **    ***  ***
  0 **************************
  0 **************************
... abcdefghijklmnopqrstuvwxyz

Where the graph represents the amount of times a letter appears. (If it is more than 10, i simply put a '+' after the 10th row). The code I've currently written to achieve this is as follows: (Haven't found a good way to test for capital letters as well as lowercase yet).

```
#include
#include
#include
#include

void drawGraph(int letters[26], char alpha[26]);
void printLetters(int letters[26], char alpha[26]);
void getLetters(FILE *fp, int letters[26], char alpha[26]);

int main(int argc, char *argv[]) {
FILE *fp;
int letters[26] = { 0 };
char alpha[26] = { 'a','b','c','d','e','f','g','h','i','j','k','l','m','n','o','p','q','r','s','t','u','v','w','x','y','z' };
int indexedAlpha[256] = { 0 };
int j = 1;

for (i = 97; i = 0)
{
y = 0;
while (y 10)
{
printf("+");
letters[i] = 10;
y++; // Break out of while loop
}
else if(letters[i] == x)
{
printf("*");
}
else
{
printf(" ");
}
if (letters[i] == x && y == 1)
{
letters[i

Let just say when I first read the question I was very impressed by the graph.

Well done.

Small mistake in reading the file:

char c;
for (int i = 0; !feof(fp); i++)
{
    c = fgetc(fp);

This is wrong in all languages. The eof is never set until you read past the eof. The last successful read reads up-to (but not past) the eof. So even if there are no more characters in the file the eof flag is not set (until you try and read the character after eof).

As a result you are suffering a one off error. The loop is executed one time to many. The value of 'c' on the last iteration is EOF truncated to fit into a char.

So the standard patter is to read from the file and see if it works. If it works then you enter the loop:

int c;
for (int i = 0; (c = fgetc(fp)) != EOF; i++)
{

Note we need to change the c from a char into an int to make sure that EOF is not truncated out of the value.

Not an error: But you are implementing an O(1) operation is O(n).

for ( int j = 0; j < 26; j++ ) //find which letter it is
    {
      if( c == alpha[j] ) 
    {
      letters[j]++;
      break;
    }

With a little though you can invert the array. So you use the letter to look up its ID.

char alpha[256] = 
{  0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   //  0->15 ignore
   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   // 16->31 ignore      
   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   // 32->47 ignore      
   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   // 48->63 ignore      
   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   // 64->79 ignore      
   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   // 80->95 ignore      
   0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,   // 96->111 a -     
   16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 0, 0, 0, 0, 0,   // 112->127 -z     
   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,   // 128->143 ignore  

   .. etc

   0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};  // 240->255 ignore      

   // Now the lookup becomes:
   if (alpha[c] != 0) 
   {
       letters[alpha[c] - 1]++;
   }

But do you really care if you count all the characters. I would not (unless there are some serious space constraints). You can just count all the characters. When you print them out you just print the ones you want.

int letters[256];

   ....
   letters[c]++;   // or maybe letters[tolower(c)]++;
   .....

   // Now we just need to de reference the count of the letters we are interested in.
   for( int i = 0; i < 26; i++ )
   {
       int count = letters[alpha[i]];
       if(count != 0){
           printf("The letter '%c' occurs %d times.\n", alpha[i], count);
       }
   }

Some small tidy up (you seem to have embeded tabs that mess up spacing on this site).

StackExchange Code Review Q#19019, answer score: 7