So I am trying to write code that will allow me to count the number of letters a user has entered. My code runs well if I simply type one word. The moment that I include any character that is not a letter, my terminal stops working.

What can I do to fix it?

#include <cs50.h>
#include <stdio.h>
#include <ctype.h>

int count_letters(string text);

int main(void)
{
    string text = get_string("text? ");
    int letters = count_letters(text);

    printf("%i letters\n", letters);
}

int count_letters(string text)
{
    int i = 0;
    do
    {
        if (isalpha(text[i]))
        {
            i  ;
        }
        else if (isspace(text[i]))
        {
            i =   0;
        }
        else
        {
            i =  0;
        }
    } while (text[i] != 0);

    return i;
}

CodePudding user response：

i is the do ... while counter, do not use it to count the number of letters, use another variable, something like:

int count_letters(string text)
{
    int i = 0, letters = 0;
    do
    {
        if (isalpha((unsigned char)text[i]))
        {
            letters  ;
        }
        i  ;
    }
    while (text[i] != 0);
    return letters;
}

Notice that with your approach the loop is also testing the NUL terminator in an empty string, use a while loop instead to exit the loop as soon as the NUL is found:

int count_letters(string text)
{
    int letters = 0;
    while (*text)
    {
        if (isalpha((unsigned char)*text))
        {
            letters  ;
        }
        text  ;
    }
    return letters;
}

CodePudding user response：

Consider what happens in your loop: i only increments when the character at text[i] is an alphabetic character. Otherwise it resets to zero.

Thus, the loop will never complete for a string that is not entirely alphabetic characters because the loop will "reset" to the beginning with ever non-alphabetic character.

We can increment an index from zero until the character at that index in the string is the null-terminator of zero (which is false in C).

int count_letters(string text) {
    int count = 0;

    for (int i = 0; text[i]; i  ) {
        if (isalpha(text[i])) count  ;
    } 

    return count;
}

We can however, use pointers to iterate over your string, taking advantage of the detection of the null terminator to terminate the loop. If each character is an alphabetic character, increment a counter. A for loop can handle giving you a temp pointer and initializing it, testing for termination of the loop and incrementing the pointer.

int count_letters(string text) {
    int count = 0;

    for (char *ch = text; *ch; ch  ) {
        if (isalpha(*ch)) count  ;
    } 

    return count;
}

As text is a pointer passed by value (being hidden by CS50's string typedef), it can be modified without affecting the original string as long as we don't dereference and modify the individual characters, so we can avoid the extra char pointer.

int count_letters(string text) {
    int count = 0;

    for (; *text; text  ) {
        if (isalpha(*text)) count  ;
    } 

    return count;
}

CodePudding user response：

As pointed out in the comment, i cannot serve two purposes (index and counter) except for the special case of a string comprised of only letters.

Below is a compact function that counts 'hits' when isalpha() has returned a non-zero value (indicating the current letter is in the range "A-Za-z").

The unusual "!!" transforms the non-zero positive value into C's true (or false) having values of 1 (or 0) respectively. Thusly, the value of 1 or 0 is appropriately added to the accumulator to be returned to the caller.

int count_letters( string text ) {
    int i = 0, cnt = 0;

    if( text != NULL ) // trust is good, testing is better
        while( text[i] )
            cnt  = !!isalpha( (unsigned char)text[i  ] );

    return cnt;
}

EDIT and Credit: @Aconcagua for pointing out the need for casting each char to unsigned char to avoid UB.

CodePudding user response：

value is nil sonil Value Is Blank

string text = get_string("(text?) ?? ("") ") int letters = count_letters((text) ?? Min);

mark:- min -> Value Nil In Set Value For Deflaut value