Using strchr() to count occurrences of a character in a string

Question

I'm almost finished with the class semester, and I'm working on an assignment to write a function to find the number of a certain character in a string, given the function prototype the teacher assigned. I know I must be doing something stupid, but this code is either locking up or looping indefinitely in my function.

It is an assignment, so I'm not looking for anyone to do my homework for me, but merely to point out where I'm wrong and why, so I can understand how to fix it. I would appreciate any help that you are willing to give.

Here's the code I've written:

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

int charCounter(char* pString, char c);

int main(void)
{
    char* inpString = "Thequickbrownfoxjumpedoverthelazydog.";
    int charToCount;
    int eCount;

    eCount = 0;
    charToCount = 'e';
    eCount = charCounter(inpString, charToCount);
    printf("\nThe letter %c was found %d times.", charToCount, eCount);

    return 0;
} // end main

int charCounter(char* pString, char c)
{
    int count = 0;
    char* pTemp;

    do
    {
        pTemp = strchr(pString, c);
        count++;
    }
    while(pTemp != NULL);

    return count;
} // end countCharacter

Solution 1

Your loop is always looking from the beginning of pString, and is always finding the first 'e' over and over again.

If you declare char* pTemp = pString; then you can iterate a little differently (I pasted the wrong version earlier, sorry!):

char* pTemp = pString;

while(pTemp != NULL)                                                                                                    
{                                                                                                                       
    pTemp = strchr(pTemp, c);                                                                                                           
    if( pTemp ) {
        pTemp++;
        count++;
    }                                                                                                
}

This forces pTemp to point just after the character you just found before looking for the next one.

It would be easier to just do:

char* pTemp = pString;
while( *pTemp )
    if( *pTemp++ == c) count++;

Okay, after thinking about it, even after you already have this working, I changed the inner loop to a form I am more happy with:

while( (pTemp = strchr(pTemp, c)) != NULL) {                                                                                                                       
   count++;                                                                                                             
   pTemp++;
}

Solution 2

You are always restarting from the beginning. No wonder you never come to an end.

char* pTemp; // Init here = pString
do {
    pTemp = strchr(pString, c); // pString? Really? Should be pTemp
    count++;
} while(pTemp != NULL); // pTemp != NULL is verbose for pTemp here

Still, better avoid the library function and do a direct loop over all elelemts.

Just to jump on the wagon train:

size_t count(const char* s, char c) {
    size_t r = 0;
    for (; *s; ++s)
        r += *s == c;
    return r;
}

If you insist on using strchr():

size_t count(const char* s, char c) {
    size_t r = 0;
    while ((s = strchr(s, c)))
        ++r;
    return r;
}

Solution 3

strchr returns a pointer to the first occurrence of the character c in pString.

So if you give a pointer to the beginning of your string in each loop, pTemp will always have the same value and never be NULL if the character c exists. That's why you have an infinite loop.

You might want to do some pointer arithmetic to solve your problem here ;)

Solution 4

strchr() is always looking in the same place, it will not progress through the string...

Try this modification to traverse through the string using length of string, and a simple char comparison:

int i, len = strlen(pString);
count = 0;
for(i=0;i<len;i++)
{
    if(pString[i] == c) count++; //increment count only if c found
}

return count;  

Without using strlen() (to address comment)

i=-1, count = 0;
while(pString[++i])
{
   if(pString[i] == c) count++;
}  
return count;

View more solutions

Comments

[JohnH]

why use strlen()? That adds an additional iteration through the string just to get its length. Delete variable len entirely and instead of i<len use pString[i]!='\0' (since all strlen does is look for a trailing NUL as well).

[ryyker]

@JohnH - Using strlen() is just one method. It works, so does yours. I am not trying to break the land speed record here, just answer the question. :) (See edit)

[David Peterson Harvey]

I love your second suggestion but the chapter is all about the use of the functions. The first answer gives a segmentation fault and dumps the core but iterating pString gets closer. However, I need to figure out how to iterate farther after the character is found because I'm getting a lot of duplicate counts. Thanks for getting me closer.

[JohnH]

Did you remember to put char* pTemp = pString; instead of char* pTemp; before my first block? :) I actually tested the block before posting it...

[David Peterson Harvey]

I would love to do that but this chapter is all about the string functions. (sigh)

[ryyker]

Thanks! Hope it helps.

[David Peterson Harvey]

Yes, and it still kicked out on me. I even tried copying and pasting your code when all else failed, in case I made some stupid little mistake. I'm compiling on an Ubuntu machine, if that makes a difference.

[JohnH]

@DavidPetersonHarvey -- I pasted the wrong version earlier, sorry. :(

[David Peterson Harvey]

Ooh, John, awesome! It just needed one small change. I put count++ with pTemp++ in a block under the if statement so it only incremented when the character was actually found. That prevented it from automatically counting the extra time before the first character was found. Perfect! Thank you! Thanks to everyone here, I understand much better how it works!

[JohnH]

I just re-wrote the loop to a form I am even more happy with and added it at the very end of my post...

[ryyker]

@JohnH - regarding the last edit, it looks like count will increment each time pTemp does. How will that result in an accurate count of c? (I see it does, just not sure how)

[JohnH]

If, and only if, pTemp wasn't null, we enter the body of the while..loop. This means we found the char, so we increment count. Since we then want to look for the next character, we increment pTemp to point to the character after the one we just found... and then the top of the while..loop goes and searches for it with the next call to strchr() starting one char after the one we just found. If it finds another, we enter the body of the while loop again, if not, we don't re-enter it.

[Dandymon]

far better and quicker method!