patterncMinor
Print the frequency of characters as a horizontal histogram
Viewed 0 times
thefrequencyhistogramprinthorizontalcharacters
Problem
Well, I've used some of the feedback from my last posting to improve how I wrote this program. Mainly using the void parameter in main, initializing my int array to zero without using a for-loop, and better naming of variables. If there are some issues that are going on with my code, or there's some things I can do better, please let me know.
Any feedback is welcome!
Any feedback is welcome!
#include
/* print histogram of the frequencies of different characters in its input */
int main(void)
{
int histogram[95] = {0}; /* initialize 95 spaces for ASCII characters 32 - 127 */
int c, i, j;
while ((c = getchar()) != EOF) {
++histogram[c-' '];
}
for (i = 32; i < 127; ++i) {
printf("%c ", i);
++histogram[c-' '];
for (j = 0; j < histogram[i-' ']; ++j) {
putchar('x');
}
putchar('\n');
}
}Solution
When dealing with offset arrrays (using position 0 to indicate character 32), it is important for readability to use symmetrical access patterns for the process. In your case, you load the values as
You also have no handling for out-of-bound characters.... can you trust your input sources to not have a carriage return, or a tab? Even esoteric null characters, or other binary values. I would have a catch-all 'bucket' for invalid values. That will prevent segfaults or other undefined actions.
You have a small bug in your code. In the print-loop, you increment the last character received.... what is this line for in the 'print' loops:
Finally, for code reuse, it is often important to extract your functionality to a single reentrant method, other than the main method. Consider a signature like:
where your histogram can be configured to take an offset position, and range, like:
which will count just the lower-case letters...
Something like:
c-' ', but what you use to read them, is accessed by i = 32; i < 127 .... and this asymmetrical access is inconvenient.You also have no handling for out-of-bound characters.... can you trust your input sources to not have a carriage return, or a tab? Even esoteric null characters, or other binary values. I would have a catch-all 'bucket' for invalid values. That will prevent segfaults or other undefined actions.
You have a small bug in your code. In the print-loop, you increment the last character received.... what is this line for in the 'print' loops:
++histogram[c-' '];Finally, for code reuse, it is often important to extract your functionality to a single reentrant method, other than the main method. Consider a signature like:
void histogram(const int offset, const int range)where your histogram can be configured to take an offset position, and range, like:
histogram('a', 'z' - 'a' + 1);which will count just the lower-case letters...
Something like:
#include
#include
void histogram(const int offset, const int range) {
int histogram[range];
memset(histogram, 0, sizeof(histogram));
int special = 0;
int c;
while ((c = getchar()) != EOF) {
if (c = (offset + range)) {
special++;
} else {
++histogram[c - offset];
}
}
for (int i = 0; i < range; ++i) {
c = i + offset;
printf("%c ", c);
for (int j = 0; j < histogram[i]; ++j) {
putchar('x');
}
putchar('\n');
}
printf("- ");
for (int j = 0; j < special; j++) {
putchar('x');
}
putchar('\n');
}
int main(void) {
histogram(' ', 95);
}Code Snippets
++histogram[c-' '];void histogram(const int offset, const int range)histogram('a', 'z' - 'a' + 1);#include <stdio.h>
#include <memory.h>
void histogram(const int offset, const int range) {
int histogram[range];
memset(histogram, 0, sizeof(histogram));
int special = 0;
int c;
while ((c = getchar()) != EOF) {
if (c < offset || c >= (offset + range)) {
special++;
} else {
++histogram[c - offset];
}
}
for (int i = 0; i < range; ++i) {
c = i + offset;
printf("%c ", c);
for (int j = 0; j < histogram[i]; ++j) {
putchar('x');
}
putchar('\n');
}
printf("- ");
for (int j = 0; j < special; j++) {
putchar('x');
}
putchar('\n');
}
int main(void) {
histogram(' ', 95);
}Context
StackExchange Code Review Q#104082, answer score: 2
Revisions (0)
No revisions yet.