How can I fix my program to not crash if I run any character that is not alphabetical?

Sizwe 0
Viewed 79

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;
}
cs50 c
4 Answers

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;
}
David Ranieri0

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;
}
userRb2210

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.

Fe2O30

When scanf is processing characters for computing the value to store in b, it will stop at the first character which cannot be part of an int. With an obedient typist, this will be the \n produce by the ENTER key but it could be any other non-digit (alpha, imbedded +, etc). The remove all these extraneous characters from the input buffer simply code while (getchar() != '\n') ; after the call to scanf. This will remove one character at a time from the input buffer until the character just removed is the one produce by ENTER.

5943__Rutvik__ Dhameliya0
Related