CS50x Readability improvement I am brand new to writing my own code. I just started the CS50x intro class and have so far made it to pset 2 readability. It's getting easier to understand what's going on. I just wanted to get some opinions on the code that I wrote to see if there were possible improvements/changes that I could make.

#include &lt;stdio.h&gt;
#include &lt;cs50.h&gt;
#include &lt;string.h&gt;
#include &lt;ctype.h&gt;
#include &lt;math.h&gt;


//initialize lenght function
int length(string str, char check);

int main(void)
{
    //Get user input
    string input = get_string("Text: ");

    //Calculate and set grade level. Take user input and run it thorugh the lenght function. l for letters, w for words and s for sentances.
    float gradeLevel = (0.0588 * (100 * (float) length(input, 'l') / (float) length(input, 'w')) - 0.296 * (100 * (float) length(input, 's') / (float) length(input, 'w'))) - 15.8;

    //output grade level
    if (round(gradeLevel) &gt;= 16)
    {
        printf("Grade 16+\n");
    }

    else if (round(gradeLevel) &lt; 1)
    {
        printf("Before Grade 1\n");
    }

    else
    {
        printf("Grade %i\n", (int) round(gradeLevel));
    }


    //print lengths for testing
    /*
    printf("%i letter(s)\n", length(input, 'l'));
    printf("%i word(s)\n", length(input, 'w'));
    printf("%i sentance(s)\n", length(input, 's'));
    */
}

//function to get length of letters, words and sentences
int length(string str, char check)
{
    int letterCount = 0;
    int wordCount = 1;
    int senCount = 0;
    int n = strlen(str);

//check letter count
    if (check == 'l')
    {
        for (int i = 0; i &lt; n; i++)
        {
            if (isalpha(str[i]))
            {
                letterCount++;
            }
        }
        return letterCount;
    }
//check word count
    else if (check == 'w')
    {
        for (int i = 0; i &lt; n; i++)
        {
            if (str[i] == ' ' || str[i] == '\n' || str[i] == '\t')
            {
                wordCount++;
            }
        }
        return wordCount;
    }
//check sentance count
    else if (check == 's')
    {
        for (int i = 0; i &lt; n; i++)
        {
            if (str[i] == '.' || str[i] == '!' || str[i] == '?')
            {
                senCount++;
            }
        }
        return senCount;
    }
    return 0;
}


Thank you for the detailed feedback. Here's the specs of the program

Your program should count the number of letters, words, and sentences in the text. You may assume that a letter is any lowercase character from a to z or any uppercase character from A to Z, any sequence of characters separated by spaces should count as a word, and that any occurrence of a period, exclamation point, or question mark indicates the end of a sentence.

Your program should print as output "Grade X" where X is the grade level computed by the Coleman-Liau formula, rounded to the nearest integer.

If the resulting index number is 16 or higher (equivalent to or greater than a senior undergraduate reading level), your program should output "Grade 16+" instead of giving the exact index number. If the index number is less than 1, your program should output "Before Grade 1".

The algorithm used was the Coleman-Liau index
You're off to a great start!  This is reasonably readable, and I'm sure you'll get better with practice.



lenght


Typo.  I make tons of typos myself.  I highly recommend getting an IDE with a spell checker.



string input = ...


This confused me at first.  It has to be a typedef, but what kind of typedef makes sense?  Turns out in &lt;cs50.h&gt;, they typedef char *string;.  This is a little weird to me because char* is a very familiar type -- I instantly know a few things to look for/what operations you can do with that.  string is an unknown entity.  I think char* input = ... is clearer.



float gradeLevel = (0.0588 * (100 * (float) length(input, 'l') / (float) length(input, 'w')) - 0.296 * (100 * (float) length(input, 's') / (float) length(input, 'w'))) - 15.8;


This line is so long!  Breaking it down and simplifying a bit:

float l_over_w = (float) length(input, 'l') / (float) length(input, 'w');
float s_over_w = (float) length(input, 's') / (float) length(input, 'w');
float gradeLevel = (5.88 * l_over_w - 29.6 * s_over_w) - 15.8;



I have no idea where the numbers 5.88, 29.6, and 15.8 came from.
Maybe the div function would help you instead of casting to float?  floats are slow and have a lot of idiosyncrasies... IMO they are best to avoid unless you absolutely MUST use them.
I find the general idea here non-obvious.  How about some comments?
This could be a function float gradeLevel(char* input);




int length(string str, char check)


You've used check to determine what this function should do.  That's generally a bad idea, but if you cannot avoid it, at least assert(false) if check is invalid.  That way you get a runtime error if check is wrong.  As you've written it, you'll silently get the wrong answer if check is wrong.

In this case, it is trivial to do better.  You could have several functions: int letterCount(char* str);, int wordCount(char* str);, and int sentenceCount(char* str);.  This way you get a compiler error if you write the wrong function name (and you can't get check wrong since check doesn't exist).



int letterCount = 0;
int n = strlen(str);
for (int i = 0; i &lt; n; i++) {
    if (isalpha(str[i])) {
        letterCount++;
    }
}
return letterCount;


This is OK, but you go over the string once to compute strlen and once to compute letterCount.  You can do them at the same time:

int letterCount = 0;
for (int i = 0; str[i]; i++) {
    if (isalpha(str[i])) {
        letterCount++;
    }
}
return letterCount;


I think this is also nicer since you don't have an n to worry about.  You could further simplify this to get rid of the index and just have the pointer:

while (str) {
    letterCount += isalpha(*str++);
}


but maybe that's overkill.

N.B. at least you didn't write for (int i = 0; i &lt; strlen(str); i++) which goes over the string once per character!



(str[i] == ' ' || str[i] == '\n' || str[i] == '\t')


How about isspace(str[i])?



(str[i] == '.' || str[i] == '!' || str[i] == '?')


How about ispunct(str[i])?  Not an exact match so be careful.

Here's a list of similar functions available https://linux.die.net/man/3/ispunct.



//print lengths for testing
/* ...


This is totally fine for a personal project, but there are lots of testing frameworks that will always compile your test code but only run it under certain conditions.  Might be worth researching.  Here's a list to get you started https://en.wikipedia.org/wiki/List_of_unit_testing_frameworks#C.



Do you need to deallocate the string returned by get_string?



Is the number of words equal to the number of spaces?  What about this string with         lots of white-spaces.....  How many sentences are in that string?  I don't know the answer to these questions (maybe the exercise tells you what to compute?), but I think it's interesting to think about.