Simple Vertical Histogram in C

Question

The exercise was to print a histogram of the different characters in the input. At the bottom is my code where I break the different characters into 3 categories a,b, or other, to extrapolate to more categories of characters if the code is successful. But it doesn't have the output I want, for example, for input ab cd I expect

  x
xxx
---
abo
  t
  h
  e
  r

but all I get is

---
abo
  t
  h
  e
  r

Here's my attempt:

#include<stdio.h>

main()
{
int i,c,b, max;
int array[3] = {0,0,0};


while( (c = getchar()) != EOF){
    if(c== 'a')
    ++array[0];
    else if(c== 'b')
    ++array[1];
    else if(c=='\n' ||c=='\t' || c==' '){}
    else ++array[2];
    }

array[0]=max;
i=0;

 while(i<3){
    if(max>=array[i])
        {}
    else array[i]=max;

    ++i;
 }

i=0;
max=b;
while(i<b){
    if(array[0]>=max){
        putchar('x');}
    else putchar(' ');


    if(array[1]>=max) {
        putchar('x');}
    else putchar(' ');


    if (array[2]>=max){
        putchar('x\n');}
    else putchar(' \n');
    --max;
    ++i;


    }

printf("---\nabo\n  t\n  h\n  e\n  r");


}

The assignments `array[0]=max;` and `else array[i]=max;`should probably be turned the other way around (max=...). You want to assign the value to max and not the other way around. Similarly the line `max=b` should be changed to `b=max` since you want to set the variable _b_ to the current value of _max_ and not the other way around. — Ma3x, Jul 12 '13 at 22:11
I recommend that you break your code into smaller functions. This makes it easier to see what each part is doing. — Code-Apprentice, Jul 12 '13 at 23:09

score 5 · Answer 1 · edited May 23 '17 at 12:21

There's a good bit wrong with your code.

Never do - main()

You should be using int main(void){...} see this answer.
You're hard coding numbers into your code. Provide constant variables instead. There are logical exceptions to this but numbers generally have meaning so state it.
unsigned const int LETTERS = 3;
You take the time to increment array[0] counting the occurrences of 'a', but then assign it the value of max directly afterwards without using that value or storing it somewhere else.
You assign array[0] after the first while(){...} but max has not been initialized, so it's "garbage".
Here -if(max>=array[i]){} you don't do anything in the body??
You assign max the value of b - again "garbage"!
You should return ...; from your main function. You'll have to see for yourself the options you have there. Note: If you follow "1." you'll have no choice but to follow this one.
Your formatting, if not in your "real" code, in the post is less than desirable (i.e.; it's not very easily read.

Fix those things and you'll probably fix the problem.

haccks · Answer 2 · 2013-07-13T10:13:50.817

Change

array[0] = max to max = array[0] (otherwise max will take some garbage value)
max = b to b = max (otherwise b will take some garbage value)
putchar('x\n') to { putchar('x'); putchar('\n')}.
(It is important to note here, that putchar() prints only one character at once but you used putchar('x\n') to print two characters x and \n together!)

then your program will give desired output.
Here is your working code with changes mentioned above:

#include<stdio.h>

int main()
{
    int i,c,b, max;
    int array[3] = {0,0,0};


    while( (c = getchar()) != EOF){
       if(c == 'a')
            ++array[0];
       else if(c == 'b')
            ++array[1];
       else if(c =='\n' || c =='\t' || c == ' ')
            ;
       else ++array[2];
    }

 max = array[0];
 i=0;

 while(i<3){
    if(max < array[i])
        max = array[i];
i++;        
}

i=0;
b = max;
while(i < b){
    if(array[0] >= max)
        putchar('x');
    else 
        putchar(' ');


    if(array[1] >= max)
        putchar('x');
    else 
        putchar(' ');


    if (array[2] >= max)
    {
        putchar('x');
        putchar('\n');
    }
    else 
        putchar('\n');
    --max;
     ++i;


  }

printf("---\nabo\n  t\n  h\n  e\n  r");

return 0;
}

Simple Vertical Histogram in C

2 Answers2