I'm solving K&R 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.
Although the exercise itself is pretty simple, I've decided to apply a more "professional" approach and encapsulate (is that a correct word?) histogram into a structure:
typedef struct {
/* Legend and character to 'draw' histogram with */
char **legend, chr;
/* Histogram data */
int *data;
/* Max. data value, bin width, space between bins, max. bin length */
int datamax, binwidth, binmargin, binlength;
/* Data and legend array lengths */
size_t datasize, legendsize;
} chart_t;
Because it's still a toy program, I decided to not overcomplicate algorithm so my histogram is limited to only integer data (it's hard to represent floating point values graphically in console anyway).
Overall algorithm works more or less correctly, so I ask to review only my chart creation procedure. The whole point of this was to learn how to use dynamic memory (allocate, free, etc.).
#define BINWIDTH_DEFAULT 3
#define BINMARGIN_DEFAULT 1
#define BINLENGTH_DEFAULT 14
/* ... */
chart_t *create_chart(int *data, size_t ds, char **lgnd, size_t ls, char chr) {
chart_t *out = (chart_t *) malloc(sizeof(chart_t));
if(out == NULL)
return NULL;
out->chr = chr;
out->datasize = ds;
out->legendsize = ls;
out->binwidth = BINWIDTH_DEFAULT;
out->binmargin = BINMARGIN_DEFAULT;
out->binlength = BINLENGTH_DEFAULT;
out->data = (int *) calloc(ds, sizeof(int));
if(out->data == NULL) {
free(out);
return NULL;
}
memcpy(out->data, data, ds);
int dmax = 0;
for(int i = 0; i < ds; i++)
if(data[i] > dmax)
dmax = data[i];
out->datamax = dmax;
char **legend = (char **) calloc(ls, sizeof(char *));
for(int i = 0; i < ls; i++) {
size_t len = strlen(lgnd[i]);
if(len == 0)
continue;
char *curstr = (char *) calloc(len+1, sizeof(char));
if(curstr == NULL) {
if(i > 1)
for(int j = 0; j < i; j++)
free(legend[j]);
free(legend);
free(out);
return NULL;
}
strncpy(curstr, lgnd[i], len+1);
legend[i] = curstr;
}
return out;
}
Haven't I missed anything? Do I have any potential memory problems in my procedure?