#Pls give me suggestions on my C code

36 messages · Page 1 of 1 (latest)

unborn jungle
#

#define MAXLEN 10  /*Maximum Word length in Histogram*/

int main()
{
    int word[MAXLEN + 1]; /*To increment values of words upto max length*/
    int counter = 0;
    int i, c, temp, max;

    printf("Enter the text input for which you want to print a Histogram with word length frequencies!\nINPUT:\n");

    for (i = 0; i < (MAXLEN + 1); ++i)
    {
        word[i] = 0;
    }
    
    while ( (c = getchar()) != EOF)
    {
        if (c == '\n' || c == '\t' || c == ' ')
        {
            temp = counter;
            
            if (temp > MAXLEN)
            {
                ++word[MAXLEN];
            }
            else
            {
                ++word[temp - 1];
            }
            counter = 0;
        }
        else
        {
            ++counter;
        }
    }
    
    for (i = 0; i < (MAXLEN + 1); ++i)
    {
        if ( max < word[i])
        {
            max = word[i];
        }
        else
        {
            continue;
        }
    }
    
    printf("\n");
    
    while (max != 0)
    {
        printf("%4d|   ", max);
        for (i = 0; i < (MAXLEN + 1); ++i)
        {
            if (word[i] >= max)
            {
                printf("*   ");
                --word[i];
            }
            else
            {
                printf("    ");
            }
        }
        printf("\n");
        --max;
    }
    
    printf("    =");
    for (i = 0; i < (MAXLEN + 1); ++i)
    {
        printf("====");
    }
    printf("\n");
    printf("     ");
    for (i = 0; i <(MAXLEN + 1); ++i)
    {
        if (i < 10)
        {
            printf("%4d", i+1);
        }
        else
        {
            printf("%4d<", 10);
        }
    }
    printf("\n");\
    return 0;
}```
This is a C program to Print a Vertical histogram for word frequencies of input text
storm roseBOT
#

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question run !howto ask.

unborn jungle
#

Can someone suggest me how to improve this code

#

I wrote this when I was absolute newb

snow galleon
#
    int i, c, temp, max;
...
    for (i = 0; i < (MAXLEN + 1); ++i)

just use

for(int i = 0; ...)

theres no reason to not do in inside the for scope

unborn jungle
#

ohk

snow galleon
#

you only use the temp inside one while loop

#

hence you can also declare it there

#
            int temp = counter;
#

max is not initialized

        if ( max < word[i])
        {
            max = word[i];
        }

here you check it before it gets assigned a value

unborn jungle
#

oh

#

I should have assigned it 0 i guess

snow galleon
#
    while ((c = getchar()) != EOF) {
        if (c == '\n' || c == '\t' || c == ' ') {
            temp = counter;
            if (temp > MAXLEN) {
                ++word[MAXLEN];
            } else {
                ++word[temp - 1];
            }
            counter = 0;
        } else {
            ++counter;
        }
    }

remove empty lines
and dont place opening brackets on a new line
it keeps it more concise and you have more code visible on your screen
but thats just my preference

unborn jungle
#

I see

#

well that initialization of max fixed the issue of unnecessary lines being printed

#

thanks

#

Anything else?

snow galleon
#
    for (i = 0; i < (MAXLEN + 1); ++i)
    {
        if ( max < word[i])
        {
            max = word[i];
        }
        else
        {
            continue;
        }
    }

the else case is redundant

#
    for (i = 0; i < (MAXLEN + 1); ++i)
    {
        word[i] = 0;
    }

can be replaced by

nt word[MAXLEN + 1] = {0};
lunar swan
#

could probably break some sections into a function, such as getting input. That way you also are fitting more code on your screen and its easier to understand yourself.

unborn jungle
#

I see

#

Fixed that too

lunar swan
#

its basically memset

#

you should probably make more use of libraries and use strtok, etc

unborn jungle
#

I am actually a beginner right now

#

this problem is a exercise in K&R book

snow galleon
#
int arr[3] = {1, 2, 3};   // [0] = 1   [1] = 2   [2] =  3
int arr[3] = {1};         // [0] = 1   [1] = 0   [2] =  0
unborn jungle
#

which i solved about a month ago

#

So yeh I am noob

#

But I am trying to get better

lunar swan
#

you will probably be dealing with strings often then so its good to learn to read the man pages for the standard library

unborn jungle
#

Ohk

#

got it

storm roseBOT
#

@unborn jungle Has your question been resolved? If so, run !solved :)

unborn jungle
#

!solved