views:

535

answers:

4

Here is my code for finding a sequence in a string and replacing it with another:

std::string find_and_replace( string &source, string find, string replace )
{
    size_t j;
    for ( ; (j = source.find( find )) != string::npos ; )
    {
     source.replace( j, find.length(), replace );
    }
    return source;
}

Everything works fine when I call something like:

find_and_replace(test, "foo", "bar")

My application requires me to replace a single quote with two single quotes, not a double quote. For example I would call:

find_and_replace(test, "'", "''")

But whenever I call this, the function freezes up for some reason. Does anyone know what might be the cause of this problem?

Edit: based on the answers I've gotten, I have fixed the code:

std::string find_and_replace( string &source, string find, string replace )
{
    string::size_type pos = 0;
    while ( (pos = source.find(find, pos)) != string::npos ) {
     source.replace( pos, find.size(), replace );
     pos += replace.size();
    }
    return source;
}

I hope this helps some people having the same problem.

+4  A: 

Because you replace ' with '', then search for ' again, finding the first of the ones you've just put there. Which you replace. And so on.

Steve Jessop
+9  A: 

You've got an infinite loop because your condition doesn't move forward. You're always running j = source.find( find ), but you're replacing ' with '', so you're always finding the first apostrophe every time and adding a new apostrophe to the string.

You need to make sure you don't match the same apostrophe twice by moving where you're scanning forward every time you replace something.

The find function takes a second parameter which is the starting position in the string to look for the substring. Once you've found the position of the first match, move the starting position up to that position plus the length of the string you're replacing it with.

Welbog
+1  A: 

You are trying to replace the same string that you added.

Ankit
+1  A: 

It's probably better to work from right to left. This works for me:

const std::string& replacestring( std::string& strString, const std::string& strOld, const std::string& strNew )
{
    for ( int nReplace = strString.rfind( strOld ); nReplace != std::string::npos; nReplace = strString.rfind( strOld, nReplace - 1 ) )
    {
        strString.replace( nReplace, strOld.length(), strNew );
        if ( nReplace == 0 )
            break;
    }
    return strString;
}
Alan
Right to left makes no difference, except accidentally.
David Thornley