C : Function Declaration

Question

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

 int count_letters(string text[]);
 int main(void)
 {
    string text= get_string("Please enter text: ");
    printf("%i Letter(s)  \n",countl);
 }
 int count_letters(string text[])
 {
    int countl= 0;
    for (int i = 0, n = strlen(text); i < n; i++)
    {
      if (isalpha(text[i]) != 0)
      {
        countl++;
      }
  }
  return countl;
  }

Hello everyone, I consulted my textbook and various educational resources on "How to declare a function in C" and I still can't make it work. In this code I want the function count_letters to return the value "countl" which I can then print on the screen. Please help me to understand how to declare this function correctly.

Answer 1

Your program has two main issues.

1.

The parameter text of count_letters is of wrong type. string is an cs50.h alias for char *. text needs to be either of type string or char * or char [] which is in the case as a function parameter equivalent to char *.

Use either

int count_letters (string text)

or

int count_letters (char* text)

or

int count_letters (char text[])

but not

int count_letters (string text[])

With string text[] text would be of type char **, which is wrong since you don't want to modify the pointer itself in count_letters.

Optionally, You also can make the pointer and/or the pointed object const to ensure no misuse or overwrites.

2.

You never call the function count_letters in main. You need to call it to get its returned value.

 int main(void)
 {
    string text = get_string("Please enter text: ");
    printf("%i Letter(s)  \n", count_letters(text));
 }

---

Side Notes:

• You should cast the argument to isalpha() to unsigned char to ensure no undefined behavior occurs when not passing an appropriate character.

• n = strlen(text); i < n; inside of the for loop in count_letters can be replaced by just the condition text[i] != '\0' or even more simpler text[i].

• Since you don't intend to provide a negative return value, you could use a return type of unsigned int or size_t for check_letters. You need to change the format specifier in the printf() call in main too to either %u for unsigned int or %zu for size_t.

---

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

 unsigned int count_letters (const string text);

 int main (void)
 {
     const string text = get_string("Please enter text: ");
     putchar('\n');         
     printf("%u Letter(s)  \n", count_letters(text));
 }

 unsigned int count_letters (const string text)
 {
     unsigned int countl = 0;
     for (unsigned int i = 0; text[i] ; i++)
     {
         if (isalpha((unsigned char)text[i]))
         {
             countl++;
         } 
     }

     return countl;
 }

Execution:

./a.out
Please enter text: hello
5 Letter(s)  

Online Test

Answer 2

looking at cs50.h header to see how string is defined:

typedef char *string;

so

int count_letters(string text[]);

defines an array of pointers of char.

When indexing text[i] the compiler returns a string when functions like isalpha expect a char. There's one dimension too much in the input.

You need a mere pointer on chars (constant is better too). Without cs50 includes, since count_letters is not modifying the text data, I would write:

int count_letters(const char *text);

so if you really want to use cs50 types, go for:

int count_letters(const string text);

Answer 3

As string is an alias for the type char * then this function declaration

int count_letters(string text[]);

is equivalent to the declaration

int count_letters( char * text[] );

that is in turn equivalent to

int count_letters( char ** text );

So within the function for example these expressions

strlen(text) 

and

isalpha(text[i])

are incorrect because in the first expression the used argument has the type char ** instead of the type char * and in the second expression the argument has the type char * instead of char.

Moreover the call of strlen is redundant.

The function should be declared and defined the following way

size_t count_letters( string text );

and

size_t count_letters( string text )
{
    size_t countl = 0;

    for ( ; *text; ++text )
    {
        if ( isalpha(( unsigned char )*text ) ) ++countl;
    }

    return countl;
}

Pay attention to that it would be more correctly to declare the function the following way

size_t count_letters( const char *text );

because the passed string is not changed in the function. To write const string text does not make sense and is not the same as const char *

And you forgot to call the function in main. You have to write

printf("%zu Letter(s)  \n", count_letters(text ) );

Answer 4

The type of the parameter needs to match the type of the expression you’re passing - in this case, you’d be passing an expression of type string, so the prototype needs to be

int count_letters(string text)

To get that count, you have to call the function from main. You can either do that from within printf call:

printf("%i Letter(s)  \n", count_letters(text));

or you can create another variable to store that result:

string text = get_string( ... );
int len = count_letters( text );
printf( "%i Letter(s)  \n", len );

The variable countl is local to the count_letters function and is not accessible from main.

<Gratuitous Rant>

The CS50 string typedef name is a lie because what it aliases is not a string, and after seeing enough questions from people taking the CS50 course I believe it is actually a hinderance to learning C; it completely misrepresents how strings are represented and handled, and if you do any C programming outside of this course you will be completely unprepared for how string processing (and I/O in general) actually work.

Get comfortable, this is gonna take a while.

In C, a string is a sequence of characters including a zero-valued terminator. The string "hello" is represented as the sequence {'h', 'e', 'l', 'l', 'o', 0}.

Strings (including string literals like "hello") are stored in arrays of character type:

char text[] = "hello"; // array size is determined by the length of the initializer

or

char text[SOME_SIZE]; // where SOME_SIZE is large enough to store what we need
strcpy( text, "hello" );

Since strings are stored in arrays, you cannot use the = operator to assign them (outside of an initializer as shown above, but that’s only valid for a declaration). You either need to use a library function like strcpy (for arrays that contain strings) or memcpy (for arrays that contain anything else), or you need to assign each element individually:

text[0] = 'h';
text[1] = 'e';
...
text[5] = 0;

Now, under most circumstances, an expression of type "array of T" (including string literals like ”hello") will be converted ("decay") to an expression of type "pointer to T" and the value of the expression will be the address of the first element. So, if we call a function like

count_letters( text );

it’s exactly the same as writing

count_letters( &text[0] );

and what count_letters actually receives is a pointer of type char *, and we’d declare the prototype as

int count_letters( char *str ) {...}

When you write something like

char *str = "hello";

you’re assigning the address of the first character of the string to str, not the string contents themselves.

string is a typedef name, or alias, for the type char * defined in cs50.h. It is not a part of the C language or standard C library. The problem is that char * is not a string. It may point to the first character of a string. It may point to the first character of a sequence that is not a string. It may point to a single character that’s not part of a larger sequence. When we’re dealing with strings we often deal with expressions of type char *, but an expression of type char * is not, in and of itself, a string.

The get_string function (again, part of the cs50 library and not a standard C library function) performs a lot of magic under the hood to dynamically allocate an array to store the string and returns a pointer to the first element (which is what you’re actually assigning). It’s slick, it’s handy, but again, it completely misrepresents how C actually does things.

</Gratuitous Rant>