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

1. ## 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.

2. 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;
}```

3. Bump, still seeking a code review

4. 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. Originally Posted by jimblumberg
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. 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.

7. Originally Posted by whiteflags
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?

Originally Posted by whiteflags
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. Originally Posted by Evan D Williams
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. @EvanDWilliams I've started this book too. Trying too do some exercises (putting most challenging for me on github).

10. Originally Posted by Evan D Williams
[...] 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. Originally Posted by Hodor
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. Originally Posted by christop
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