2
\$\begingroup\$

I am new to programming and want to know if this is really bad code. I know my variable names may seem arbitrary but I spent some time trying to get them right. I am struggling on that. Would you do this the same way?

#include <stdio.h>
#define MAXLINE 100

void printHistogramHorizontally(int alphabetofline[]);

int main()
{
    int c,i,j;
    int alphabetofline[25];
    char line[MAXLINE];


    for(j=0; j< 26; j++)
        alphabetofline[j] = 0;


    for(i=0;i < MAXLINE -1 && (c = getchar()) != EOF && c != '\n'; ++i)
    {
        line[i] = c;

        switch (c)
        {
            case 'a':
            case 'A':
                ++alphabetofline[0];
                break;
            case 'b':
            case 'B':
                ++alphabetofline[1];
                break;
            case 'c':
            case 'C':
                ++alphabetofline[2];
                break;
            case 'd':
            case 'D':
                ++alphabetofline[3];
                break;
            case 'e':
            case 'E':
                ++alphabetofline[4];
                break;
            case 'f':
            case 'F':
                ++alphabetofline[5];
                break;
            case 'g':
            case 'G':
                ++alphabetofline[6];
                break;
            case 'h':
            case 'H':
                ++alphabetofline[7];
                break;
            case 'i':
            case 'I':
                ++alphabetofline[8];
                break;
            case 'j':
            case 'J':
                ++alphabetofline[9];
                break;
            case 'k':
            case 'K':
                ++alphabetofline[10];
                break;
            case 'l':
            case 'L':
                ++alphabetofline[11];
                break;
            case 'm':
            case 'M':
                ++alphabetofline[12];
                break;
            case 'n':
            case 'N':
                ++alphabetofline[13];
                break;
            case 'o':
            case 'O':
                ++alphabetofline[14];
                break;
            case 'p':
            case 'P':
                ++alphabetofline[15];
                break;
            case 'q':
            case 'Q':
                ++alphabetofline[16];
                break;
            case 'r':
            case 'R':
                ++alphabetofline[17];
                break;
            case 's':
            case 'S':
                ++alphabetofline[18];
                break;
            case 't':
            case 'T':
                ++alphabetofline[19];
                break;
            case 'u':
            case 'U':
                ++alphabetofline[20];
                break;
            case 'v':
            case 'V':
                ++alphabetofline[21];
                break;
            case 'w':
            case 'W':
                ++alphabetofline[22];
                break;
            case 'x':
            case 'X':
                ++alphabetofline[23];
                break;
            case 'y':
            case 'Y':
                ++alphabetofline[24];
                break;
            case 'z':
            case 'Z':
                ++alphabetofline[25];
                break;
        }


    }


    if (c == '\n')
    {
        line[i] = c;
        ++i;
    }


    line[i] = '\0';

    printf("\n%s\n", line);

    printHistogramHorizontally(alphabetofline);

    return 0;

    }

    void printHistogramHorizontally(int alphabetofline[])
    {
    char const* alphaIndex[] =  {"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 i, numOfX;
    i=numOfX=0;

    for( i = 0; i < 26; ++i )
    {
        printf("%s: ", alphaIndex[i]);

        while(numOfX < alphabetofline[i])
        {
            printf("X");
            ++numOfX;
        }
        numOfX = 0;
        printf("\n");
    }

}
\$\endgroup\$
1
\$\begingroup\$
  • A character itself can serve as index. This observation greatly simplifies the reading loop:

    for(i=0;i < MAXLINE -1 && (c = getchar()) != EOF && c != '\n'; ++i)
    {
        line[i] = c;
        if (!is_letter(c))
            continue;
        if (is_upper(c))
            c = to_lower(c);
        alphabetofLine[c - 'a'] += 1;
    }
    

    Standard C provides isalpha, isupper and tolower functions, but I suggest you to implement them for practice. They are as simple as a couple of comparisons, and some addition and subtraction.

    • alphabetofLine is not a good name. I recommend letter_histogram or something to that respect.

    • I don't see how j is used.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the feedback. I was going through K&R exercises and the book has people do programming without many functions to get the hang of it. The way you did it was much better. I will practice them. Thank you about the comment for the alphabetofline, I was having so much trouble trying to name it. Also, J is used to initialize the alphabetofline to 0. \$\endgroup\$ – John Jackson Feb 24 '15 at 1:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.