Why does my while loop never end, even while the produced integer fails the conditions to run

Viewed 89

I'm trying to do a simple while loop that searches a string for a specific character then returns each place the character appears in the string before ending the loop

Here is a version of my code, where I have been trying to catch when the point where the number is no longer either 0, or less than the length of the string, as I couldn't get the while loop to end without the if statement, I tried adding it in to force 'num' to be outside these bounds and see if the while loop conditions would catch it and end the loop, seemingly not.


    #include <iostream>
    using namespace std;
    
    int main() {
        string phrase = "Hello I am a pineapple";
        
        int phraseLength = int(phrase.length());
        int num = 0;
        
        
        while(num < phrase.length() -1 && num > -1)
        {
            num = int(phrase.find('a', num));
            cout << num << endl;
            num++;
            if(num < phraseLength -1 || num ==0){
                cout << num << endl;
            }else{
                num = 24;
            }
        
        }

I'm more used to python and currently trying to learn c++, I know if I were to use these conditions in python, the loop would end, so why not here?

Sample output:

9
11
12
17
18
-1
0
8
9
11
12
17
18
-1
0
8
9
11
12
17
18
-1
0
8
9
11
12
17
18
-1
0
8
9
11
12
17
18
-1
0
8
9
11
12
17
18
-1
0
8
9
11
12
17
18
-1
0
8
9
11
12
17
18
-1
0
4 Answers

Your loop has no exit condition because you don't test the return value of std::string::find properly.

If you read the manual, you'll see that the return value when no match is found is std::string::npos.
Don't expect it to have any other value than the constant provided by the STL, or you'll be sorry.

This will work:

#include <iostream>
#include <string>    // include what you need instead of relying on other includes
//using namespace std; <--- don't do this unless you know exactly what you're doing
//                          (which is unlikely given your limited acquantaince with C++)
int main(void) // "void" is optional, but indicates (argc,argv) have been purposely ignored
{
    std::string phrase = "a Hello I am a pineapple a"; // test edge cases when possible
                                                       // (here, the first and last chars)
    size_t position = 0;  // "num" is a terrible variable name, like "int someint"
                          // and the proper type to use is size_t (unsigned).
                          // Using an int will get you a warning.
                          // Ignore warnings at your own risk...
    while (true)
    {
        position = phrase.find('a', position);    // look for the character
        if (position == std::string::npos) break; // exit condition
        std::cout << position << std::endl;       // display found occurrences
        position++; // move to next character (or end of string)
    }
}

Or if for loops are more your thing, you can slightly reduce the code like so:

for (size_t position = 0 ; ; position++)
{
    position = phrase.find('a', position);
    if (position == std::string::npos) break;
    std::cout << position << std::endl;
}

Some people find that nicer, some harder to read.
I rather like it because it makes position local to the loop, preventing you from doing something unwise like reusing the variable outside the loop.
It's really a matter of preference.

The issue was that (like @kalyanswaroop said) when find() cannot find a character it returns -1, the next step in the loop changes the assigned value from -1 to 0, and so it satisfies the conditions of the loop. In order to prevent this from happening I changed the code to the below, so that if num is -1, it breaks the loop.

#include <iostream>
using namespace std;

int main() {
    string phrase = "Hello I am a pineapple";
    
    int phraseLength = int(phrase.length() - 1);
    int num = 0;
    
    
    while(num >= 0)
    {
        
        num = int(phrase.find(char('a'), num));
        if(num > -1)
        {
            cout << num << endl;
            num++;
        }
        else{
            break;
        }

    }
  
    return 0;

}

Suggestion: Change your code

Here is a few suggestions

Using find()


The by far easiest way to do this is by using Find() (Thanks to neko for suggesting this. Not sure if he really suggested that)

Example:

#include <iostream>
#include <string>
#include <algorithm> //Don't know if this is required
int main()
{
     string phrase = "Hello I am a pineapple";
     auto location = std::find(phrase.begin(), phrase.end(), 'a');
     cout << *location;
}

Creating a function like find()


Another way to do this (also to get a taste on how find() might be implemented) is by writting a function similar to find()

Example:

#include <iostream>
#include <string>
using namespace std;

template<typename Iterator, typename T>
Iterator Find_Character(Iterator begin, Iterator end, T Find)
{
   while (begin != end && *begin != Find) ++begin;
   return begin;
}

int main()
{
    string phrase = "Hello I am a pineapple";
    auto location = Find_Character(phrase.begin(), phrase.end(), 'a');
    std::cout << "Found character: " << *location << endl; 
}

when the phrase.find cant find 'a', it returns -1, and then you have num++ which brings num back to the same condition as the beginning. Perhaps you should do something like:

while(num < phrase.length() -1 && num > -1)
{
    int findPos = int(phrase.find('a', num));
    if(findPos != nPos) //nPos is defined as -1
    {
            num = findPos ;
    }
    else
    {
            break ;
    }
    cout << num << endl;
    num++;

} 
Related