How to count only letters in string?

Question

I am trying to write a program that counts several elements in a string. The first one of those being letters.

The assignment is part of the CS50 week 2 problem set, hence the libraries included.

Using the while condition I was able to count every single character, but the code stopped working once I´ve added isalnum (which checks if the character is alphanumerical).

What am I doing wrong?

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

int main(void) {
    string text = get_string("Text: ");
    printf("%s, \n", text);

    int letters = 0;
    while (text[letters] != '\0') {
        if (isalnum(text[letters])) {
            letters++;
        }
    }
    printf("%i \n", letters);
}

Answer 1

Here is shown how a correct loop can be defined

size_t letters = 0;
for ( size_t i = 0; text[i] !='\0'; i++ )
{
    if ( isalnum( ( unsigned char )text[i] ) )
    {
       letters++;
    }
}

printf( "%zu\n", letters );

If you want to use a while loop then it can look for example like

size_t letters = 0;
size_t i = 0;
while ( text[i] !='\0' )
{
    if ( isalnum( ( unsigned char )text[i++] ) )
    {
       letters++;
    }
}

printf( "%zu\n", letters );

Pay attention to that the function isalnum detects letters and digits. If you need to count only letters then use the function isalpha.

Answer 2

If you want to count only letters:

size_t count_letters(const char *str)
{
    size_t count = 0;

    while(*str)
    {
        count += !!isalpha(*str++);
    }

    return count;
}

or if you prefer for loop

size_t count_letters_for_loop(const char *str)
{
    size_t count = 0;

    for(; *str; str++)
    {
        count += !!isalpha(*str);
    }

    return count;
}

As David suggested:

!! operation logically negates two times the value. As any logical operation gives 0 or 1, this double negation gives 1 for any non zero value or 0 for zero. It is needed as isalpha function Returns non-zero value if c is an alphabet, else it returns 0.

try to move such a logic to the separate functions. It is a very important skill in the C language

Answer 3

You while loop has a flaw: you use the same variable to count the alphanumeric characters and to index the string. It only works if all characters are alphanumeric, otherwise you get an infinite loop because you stop incrementing letter.

You should use a for loop with an index i.

Also note that isalnum() all all functions from <ctype.h> are undefined for negative values, except EOF. On platforms where char is signed by default, some characters in the string may have negative values, causing undefined behavior if you pass them to isalnum(). You should cast char values as (unsigned char) to avoid this.

Here is a modified version.

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

int main(void) {
    string text = get_string("Text: ");
    printf("%s, \n", text);

    int letters = 0;
    for (int i = 0; text[i] != '\0'; i++) {
        if (isalnum((unsigned char)text[i])) {
            letters++;
        }
    }
    printf("%i\n", letters);
    return 0;
}

Answer 4

As you have found, since not all characters in your string are guaranteed to be alpha-number characters, your use of letters as a counter and an index is flawed. When you encounter a character that is not alpha and not a number, if (isalnum(text[letters])) tests false and letters is never incremented resulting in an infinite loop at that point. (you test the same character again on the next iteration -- with the same result -- and the scenery never changes....)

As all of the other very good and very correct answers suggest, just use a separate loop counter variable (or a pointer) and increment that to iterate over your string.

One other thing you might do to validate your logic (while outputting the isalnum() characters) is simply to do away with reprinting the original string with printf (you have that right in front of you from your entry), and instead output the characters that match your criteria. For example:

    for (int i = 0; text[i]; i++) {
        if (isalnum((unsigned char)text[i])) {
            putchar (text[i]);
            letters++;
        }
    }

That is just a slight variation on your output which serves double-duty providing your output and also providing confirmation of each character that matched the criteria used.

Also note, there is no need to #include <string.h> as there are no functions requiring its inclusion used in your code. With those changes a quick example could be:

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

int main(void) {

    int letters = 0;
    string text = get_string("Text: ");

    for (int i = 0; text[i]; i++) {
        if (isalnum((unsigned char)text[i])) {
            putchar (text[i]);
            letters++;
        }
    }

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

Example Use/Output

$ ./bin/ltrcountcs50-1
Text: 123.abc-456_def_*.*_789
123abc456def789, 15

Moving Count Into A Function

You can move your counting of alpha-num characters into a function easily. One benefit of passing your string to a function is the pointer received by the function is a copy of the pointer from main() (C is pass-by-value). That allows you to simply iterate with the parameter and return the count of alpha-num characters, e.g.

int countalnum (const char *s)
{
    int letters = 0;

    while (*s)
        if (isalnum((unsigned char)*s++))
            letters++;

    return letters;
}

(note: you must pass the parameter as const char * to use a pointer to constant char. You cannot use const string using the cs50 typedef. If you will not be changing the value passed in the function, passing as const allows the compiler to make optimization it would not otherwise be able to make)

With the function above, your code reduces to:

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

int countalnum (const char *s)
{
    int letters = 0;

    while (*s)
        if (isalnum((unsigned char)*s++))
            letters++;

    return letters;
}

int main(void) {

    string text = get_string("Text: ");

    printf ("%s, %d\n", text, countalnum(text));
}

Example Use/Output

$ ./bin/ltrcountcs50-1
Text: 123.abc-456_def_*.*_789
123.abc-456_def_*.*_789, 15

But here, using the function results in printing the entire original string in main(). You can adjust the output as desired.

Answer 5

You said being letters, and then you use isalnum? so numbers will be counted as letters in your case and you increment the value of the letters which the loop counts on to iterate over the string only when it is a letter so that leads to infinite loop when the condition is false here is a solution:

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

int countLetters(char *str) {
  int letters = 0, ind = 0;
  while(str[ind] != '\0') 
    isalpha((unsigned char)str[ind++]) && letters++;
  return letters;
}

int main(void) {
  char text[100];
  fgets(text, sizeof(text), stdin);
  printf("%i", countLetters(text));
  return 0;
}

// input: hello 15-p */x
// output: 7