user2779581
user2779581

Reputation: 169

Simple program involving a loop getting stuck for some reason

The program is getting stuck when I enter an input string. I've tested out every other branch in the program so the problem is here.

Note: The infinite loop is intentional and should be broken by the break statement.

for (i = 0 ;  i >= 0 ; i++)
{
  text.append("kk");
  if ((text.find("." , j)) < 0 )
  {
     text.erase(text.size() - 2, 2);
     text2.append(text);
     writer << text2 << endl;
     text2.clear();
     j = 0;
     break;
  }
  else
  {
     j = text.find("." , j) + 1; 
     k = j + 1;
     letter = static_cast <int> ( text.at(k) );
     if (( letter < 123 ) && ( letter > 96 ))
     {
       letter = (letter - 32);
       (text.at(k)) = static_cast <char> (letter);
       text.erase(text.size() - 1, 2);
     }
   else 
   {
     text.erase(text.size() - 1, 2); 
   }
  }
}

Upvotes: 3

Views: 1425

Answers (4)

Chimera
Chimera

Reputation: 6018

EDIT: Nevermind, my answer is wrong. I wasn't aware that std::npos is a constant set to -1.

The line: if ((text.find("." , j)) < 0 )

will never be true and so the break is never executed.

std::string.find() returns std::npos, not a value less than 0, if the text isn't found.

See: std::string.find()

Upvotes: 1

Chimera
Chimera

Reputation: 6018

I know you want to stick with your code, however, it really is pretty bad and I'd hate for you to learn bad habits.

Sometimes, even most times, we as developers have to be able to learn from code examples. I know you don't want to use any code constructs that you don't understand. However, in university and on the job you will have to learn from code you don't understand. It is a good way to improve your skill and knowledge.

I've written some code that solves your problem. It may not be the best solution. It's tested and works. Please look over this code and ask if you don't understand anything. Hopefully this will be of value to you.

#include <string>
#include <cctype>

void ToUpper( std::string& text, int position );

void ToUpper( std::string& text, int position )
{
    char c;

    c = text.at(position);
    text.at(position) = std::toupper(c);

    return;
}


int main(int argc, char *argv[])
{
    std::string text = "this is. a very simple. example. ok.";

    int found = 0;

    //Handle first character of sentence
    if ((found = text.find_first_not_of(" ", 0)) != std::string::npos)
    {
        ToUpper( text, found );     
    }

    while ((found = text.find(".", found )) != std::string::npos)
    {
        found = text.find_first_not_of(" .", found);
        if( found != std::string::npos )
        {
            ToUpper( text, found );
        }
    }

    return 0;
}

Upvotes: 0

StevieG
StevieG

Reputation: 8709

It's because you never erase the . so you never enter your first if condition (the one with the break).

Your logic looks like this:

Append "kk" to the string
If you don't find a '.'
  exit
Else
  If the second letter after the '.' is lower case
    Change it to upper case
    Delete the last letter in the string
  Else
    Delete the last letter in the string

Then you loop again

Assume your string is: zzz.abcd your iterations will be:

zzz.aBcdk
zzz.aBcdkk
zzz.aBcdkkk

etc..

this is the line doing the most of the damage:

j = text.find("." , j) + 1;

here, if you don't find the '.', you're setting j to 0 (-1 + 1) so on your next iteration, you're doing exactly the same search again.

Upvotes: 2

andre
andre

Reputation: 7249

As other have pointed out already you have an infinite loop. I normally see a string find loop in the following format.

int found = 0; 
while ((found = text.find(".", found )) != string::npos) {
    //...
}

Upvotes: 2

Related Questions