Thread: K&R Exercise 1.13 Solution - Seeking Constructive Criticism

  1. #1
    Registered User
    Join Date
    Feb 2018
    Posts
    4

    K&R Exercise 1.13 Solution - Seeking Constructive Criticism

    Hi everyone, this is my first post here

    I learned some Unix-based C back in college, worked as an IVR developer in clinical trials for 3 years, and have been working for the last 2 years as a Business Analyst. I'm picking C back up again to brush up on my technical skills and eventually get into some hobbyist embedded systems programming.

    Currently studying with K&R 2e and am working on 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"
    I decided to go the route of using a vertical histogram and am looking for some constructive feedback on the code below. As part of my implementation, it was especially challenging to orient the x-axis numbers vertically with correct spacing and place value, but I think I've finally got it.

    Note: I tried to include only features of the language that appear in chapter 1. So even though I'm aware of the existence of math.h's pow function and stdlib's exit() to implement error checking, I purposefully omitted them to focus on stdio.h and also get some practice with complex for loops.

    Code:
    #include <stdio.h>
    
    
    #define MIN_WORD_LENGTH     1    /* No error checking built in for negative numbers
                                        (have not learned stdlib yet to use exit() ...) */
    #define MAX_WORD_LENGTH     25     /* Do not exceed 9999 - y-axis padding only goes up to 4 digits */
    
    
    #define OUTSIDE_WORD         0
    #define INSIDE_WORD         1
    
    
    int main(void)
    {
        /* Initialization */
        char     c                                         = 0;
        int        state                                     = OUTSIDE_WORD,
                i                                         = 0,
                j                                        = 0,        
                wordLength                                 = 0,
                maxWordLengthFrequency                     = 0,
                xAxisWidth                                = (MAX_WORD_LENGTH - MIN_WORD_LENGTH) + 1,
                xAxisDepth                                = 0,
                xAxisLayer                                = 0, /* horizontal layer of the x-axis currently being filled */
                xAxisLayerMaxDivisions                    = 0, /* The maximum number of division operations that can be permitted
                                                                 for this layer of the x-axis */
                xAxisLayerMinPlaceValue                    = 0, /* The minimum word frequency value for which a number can possibly 
                                                                  be displayed on the current layer of the x-axis 
                                                                i.e. print a number here, or blank space? */
                xAxisCurrentNumber                        = 0, /* The number for which a digit is currently being printed on the x-axis */
                xAxisDigit                                = 0; /* Target of integer division and modulo operations to extract digits from
                                                                the numbers being printed on the x-axis */
        int     wordLengthFrequencies[MAX_WORD_LENGTH]     = { 0 };
    
    
        /* Process Input and Record Length Frequencies */
        while ((c = getchar()) != EOF)
        {
            //if (c == ' ' || c == '\t' || c == '\n' || c == '\v' || c == '\b')
            
            /* A "word" will be defined as contiguous set of alphanumeric characters  
               i.e. Words will terminate if we hit a non-alphanumeric character
                (punctuation / special characters will not count towards word lengths) */ 
            if ( ! ((c >= 'A' && c <= 'Z') 
                 || (c >= 'a' && c <= 'z')
                 || (c >= '0' && c <= '9')))
            {
                if (state == INSIDE_WORD) 
                {
                    //Leaving a word, log its length and reset word length counter
                    wordLengthFrequencies[wordLength - 1]++;
                    wordLength = 0; 
                    state = OUTSIDE_WORD; 
                }
            }
            else //Entering a word
            {
                //All words at or past the max length are counted as one category maxLength+
                if (wordLength < MAX_WORD_LENGTH) 
                { 
                    wordLength++;
                }
                state = INSIDE_WORD;
            }        
        }
        
        /* Determine Height of Histogram 
           (i.e. Max # of Occurrences) */
        for (i = MIN_WORD_LENGTH; i <= MAX_WORD_LENGTH; i++)
        {
            if (wordLengthFrequencies[i - 1] > maxWordLengthFrequency)
            {
                maxWordLengthFrequency = wordLengthFrequencies[i - 1];
            }
        }
    
    
        /* Find Necessary Depth of X-Axis Numbers 
            (integer division truncates each place value until we
             have the number of digits in the max word length) */
        i = MAX_WORD_LENGTH;
        while (i != 0)
        {
              i /= 10; 
            xAxisDepth++;
        }
    
    
        /* Print Histogram Bars and Y-Axis */
        printf("\nWord Length Frequency Histogram:\n\n"); //Histogram title
        for (i = maxWordLengthFrequency; i > 0; i--)
        {
            printf("%4d | ", i); //See preprocessing - 9999 max length for 4-digit padding
            for (j = MIN_WORD_LENGTH; j <= MAX_WORD_LENGTH; j++)
            {
                if (wordLengthFrequencies[j - 1] >= i)
                {
                    printf("x  "); //histogram bar element
                }
                else
                {
                    printf("   "); //blank space
                }
            }
            printf("\n");
        }
    
    
        /* Print Histogram X-Axis Bar */
        printf("     --");
        for (i = MIN_WORD_LENGTH; i <= MAX_WORD_LENGTH; i++)
        {
            printf("---"); 
        }
        printf("\n");
    
    
        /* Print Histogram X-Axis Numbers 
            (Most Significant Digit First) */
        
        //Iterate through each vertical layer of the X-axis numbers
        for (xAxisLayer = 1, xAxisLayerMaxDivisions = xAxisDepth; 
            xAxisLayerMaxDivisions > 0; 
            xAxisLayer++, xAxisLayerMaxDivisions--) 
        {
            printf("     ");    
            
            /* Place value will vary for digits occupying the same row 
                if the larger numbers are of different magnitudes */
            for (xAxisLayerMinPlaceValue = 1, i = 1; 
                 i < xAxisLayer; 
                 xAxisLayerMinPlaceValue *= 10, i++);
            
            /* Iterate through each length measurement and print the digit
                appropriate for the x-axis layer */
            for (xAxisCurrentNumber = MIN_WORD_LENGTH; 
                 xAxisCurrentNumber <= MAX_WORD_LENGTH; 
                 xAxisCurrentNumber++)
            {
                /* Only print a digit if the number is actually large enough
                    to reach down to this layer of the x-axis */
                if (xAxisCurrentNumber >= xAxisLayerMinPlaceValue)
                {
                    xAxisDigit = xAxisCurrentNumber;
                    for (i = xAxisLayerMaxDivisions; 
                         i > 1 && xAxisDigit >= 10 && xAxisDigit >= xAxisLayerMinPlaceValue * 10; 
                         i--)
                    {
                        xAxisDigit /= 10; //Truncate out the unneeded digits by exploiting integer/integer division
                    }
                    xAxisDigit %= 10; //Modulo 10 snags the least significant digit after divisions are applied
                    printf("  %d", xAxisDigit);
                }
                else
                {
                    printf("   "); 
                }
            }
            printf("\n");
        }
    
    
        /* Print Final + on Last Value */
        printf("    ");
        for (i = 1; i <= xAxisWidth; i++)
        {
            printf("   ");
        }
        printf("+\n");
    }
    The main thing I'm not totally crazy about having done here was the dual initialization and increment operations in my for loops. I think they might be dense and hard to follow for someone unfamiliar with my program reading it. I tried to mitigate this by using descriptive variable names and comments, but I'd be really interested to hear a different perspective on how well-documented you'd consider this.
    Last edited by Evan D Williams; 02-11-2018 at 06:19 PM.

  2. #2
    Registered User
    Join Date
    Dec 2017
    Posts
    631
    Code:
    #include <stdio.h>
    
    void print_vertical_bargraph(int *a, int len) {
        int high = a[1];
        for (int i = 2; i < len; i++)
            if (a[i] > high)
                high = a[i];
    
        for (int row = high; row > 0; row--) {
            for (int col = 1; col < len; col++) {
                if (a[col] >= row)
                    printf("   * ");
                else
                    printf("     ");
            }
            putchar('\n');
        }
    
        for (int col = 1; col < len; col++)
            printf("%4d ", col);
        putchar('\n');
    
    
    int main() {
        int freq[] = {0, 5, 3, 5, 8, 0, 3};
        print_vertical_bargraph(freq, sizeof freq / sizeof freq[0]);
        return 0;
    }
    Last edited by john.c; 02-11-2018 at 06:50 PM.
    The world hangs on a thin thread, and that is the psyche of man. - Carl Jung

  3. #3
    Registered User
    Join Date
    Feb 2018
    Posts
    4
    Bump, still seeking a code review

  4. #4
    Registered User
    Join Date
    May 2010
    Posts
    4,447
    Functions should be your friend.

    Also as posted you don't need to worry about exit() since everything is in main() all you need to do is to return to the operating system.

  5. #5
    Registered User
    Join Date
    Feb 2018
    Posts
    4
    Quote Originally Posted by jimblumberg View Post
    Functions should be your friend.
    Haha, yeah the text actually introduces those as the next topic after exercise 1-14. I just didn't use any since they hadn't appeared in the text yet.

  6. #6
    Lurking whiteflags's Avatar
    Join Date
    Apr 2006
    Location
    United States
    Posts
    9,609
    getchar() returns an int.

    And this is a nitpick, but your finite state machine has all of two states. I think the state variable can be eliminated and you'd still have something nice:

    Code:
    int freq[MAX_WORD_LENGTH] = {0, };
    int wordlength = 0;
    int c = getchar();
    while (c != EOF)
    {
        if (c == ' ' || c == '\t' || c == '\n' || c == '\v' || c == '\b')
        {
            if (wordlength >= MAX_WORD_LENGTH)
                wordlength = MAX_WORD_LENGTH;
            
            if (wordlength > 0) 
            {
                freq[wordlength - 1]++;
                wordlength = 0;
            }
        }
        else
        {
            wordlength++;
        }
        c = getchar();
    }
    Maybe if the transition were somewhat complicated, I would feel differently, but you are welcome to ignore this part, anyway.
    Last edited by whiteflags; 02-13-2018 at 08:22 PM.

  7. #7
    Registered User
    Join Date
    Feb 2018
    Posts
    4
    Quote Originally Posted by whiteflags View Post
    getchar() returns an int.
    You know, I actually did run into this when I started with the getchar()/putchar() exercises. I used char (which I assumed to be unsigned by default like K&R had explained) during an exercise just to see if it would cause EOF to freak out. Unexpectedly, it continued working just fine. I then realized that in my Pelles C environment, char is signed by default, and using unsigned char is a configuration point. So I continued using char... but it's probably not good standard practice is it? Since someone else wanting to use this code could have a compilation environment where char is unsigned by default?

    Quote Originally Posted by whiteflags View Post
    And this is a nitpick, but your finite state machine has all of two states. I think the state variable can be eliminated and you'd still have something nice:

    Maybe if the transition were somewhat complicated, I would feel differently, but you are welcome to ignore this part, anyway.
    Interesting, I didn't see that -- thanks!

  8. #8
    Registered User
    Join Date
    May 2012
    Location
    Arizona, USA
    Posts
    656
    Quote Originally Posted by Evan D Williams View Post
    You know, I actually did run into this when I started with the getchar()/putchar() exercises. I used char (which I assumed to be unsigned by default like K&R had explained) during an exercise just to see if it would cause EOF to freak out. Unexpectedly, it continued working just fine. I then realized that in my Pelles C environment, char is signed by default, and using unsigned char is a configuration point. So I continued using char... but it's probably not good standard practice is it? Since someone else wanting to use this code could have a compilation environment where char is unsigned by default?
    Yes, and also because you may read a character that has the same value as EOF when the character is converted to a signed char (character 255 has the value -1 when converted to signed char on most common systems). It's not likely to happen with text files, but it's more likely to happen with binary files.

  9. #9
    Registered User
    Join Date
    Feb 2018
    Posts
    7
    @EvanDWilliams I've started this book too. Trying too do some exercises (putting most challenging for me on github).

  10. #10
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,456
    Quote Originally Posted by Evan D Williams View Post
    [...] I used char (which I assumed to be unsigned by default like K&R had explained) [...]
    Does K&R really say that char is unsigned by default? If so it's a typo because that's not true.

    The C Standard says
    The three types char, signed char, and unsigned char are collectively called the character types. The implementation shall define char to have the same range, representation, and behavior as either signed char or unsigned char
    I.e. it's up to the compiler whether 'char' on its own is signed or unsigned. It's been this way since the first ANSI C Standard was written but who knows what went on before that.

    Maybe you have an old -- pre ANSI?? -- copy of K&R?

  11. #11
    Registered User
    Join Date
    May 2012
    Location
    Arizona, USA
    Posts
    656
    Quote Originally Posted by Hodor View Post
    Does K&R really say that char is unsigned by default? If so it's a typo because that's not true.
    Even if K&R says that char is unsigned by default, it's not really relevant at this point. C has been standardized for nearly 30 years now, which should be enough time to get up to speed with it.

    I was curious about the accuracy of that claim. I found this sentence on page 36 in the second edition (post-ANSI C) of "The C Programming Language":

    Whether plain chars are signed or unsigned is machine-dependent, but printable characters are always positive.
    And I found this on page 40 in the first edition (pre-ANSI C):

    The language does not specify whether variables of type char are signed or unsigned quantities.

  12. #12
    misoturbutc Hodor's Avatar
    Join Date
    Nov 2013
    Posts
    1,456
    Quote Originally Posted by christop View Post
    Even if K&R says that char is unsigned by default, it's not really relevant at this point. C has been standardized for nearly 30 years now, which should be enough time to get up to speed with it.

    I was curious about the accuracy of that claim. I found this sentence on page 36 in the second edition (post-ANSI C) of "The C Programming Language":



    And I found this on page 40 in the first edition (pre-ANSI C):
    Page 36's quote is interesting. I knew the machine-dependent part but didn't know that printable characters would always be positive (even though I know of know encoding where they're not! heh) Thanks christop

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Looking for constructive criticism
    By skyliner in forum C++ Programming
    Replies: 32
    Last Post: 09-15-2012, 10:56 PM
  2. Looking for constructive criticism
    By wd_kendrick in forum C Programming
    Replies: 16
    Last Post: 05-28-2008, 09:42 AM
  3. Constructive criticism/suggestions
    By LineOFire in forum C Programming
    Replies: 11
    Last Post: 09-30-2006, 09:32 AM
  4. Constructive criticism, suggestions etc
    By BobS0327 in forum C Programming
    Replies: 3
    Last Post: 01-08-2006, 09:35 AM
  5. Constructive criticism highly appreciated!
    By muggizuggi in forum C++ Programming
    Replies: 17
    Last Post: 08-01-2005, 11:54 AM

Tags for this Thread