1

I'm writing a function, which takes three parameters:

  • target : Target String
  • oldVal : Old substring
  • newVal : New Substring (To replace the oldVal)

The task of this function is to find all the occurrence of oldVal in target string, and replace them with newVal.

This is the function I've got at the moment:

std::string replace_old_with_new(std::string target, std::string oldVal, std::string newVal) {

    std::cout << "target : " << target << ", oldVal: " << oldVal << ", newVal: " << newVal << "\n";
    std::string::iterator begin = target.begin();
    std::string::iterator oldValBegin = oldVal.begin();

    while (begin != target.end()) {
        if (*begin == *oldValBegin) {
            target = target.replace(begin, begin + oldVal.size(), oldVal);
            begin = target.begin();
        } else {
            ++begin;
        }
    }

    return target;
}

The following call to the above function:

replace_old_with_new("Hello! hi hi!", "hi", "bye");

should return the string - 

"Hello! bye bye!"

But, when I run the code, nothing happens. It seems like I'm stuck in an infinite loop. The cursor keeps blinking on terminal. Is something wrong with my function. I think what might be troubling is the replace call in the if block. Is that the correct way to use iterator range in the replace function call? I can do this with erase and insert. But I want to use replace here.

Rohit Jain
  • 209,639
  • 45
  • 409
  • 525
  • possible duplicate of http://stackoverflow.com/questions/4643512/replace-substring-with-another-substring-c – alexbuisson Jul 11 '13 at 18:17
  • There's some tricky bookkeeping needed to keep track of where you are when you do an in place replacement. If the replacement text is shorter than the text being replaced you can do it fairly easily, but if the replacement text is longer than the text being replaced you have to move the tail of the string out far enough to leave room for the new text; if you do that repeatedly, you end up doing a lot of copying, which makes it really slow. – Pete Becker Jul 11 '13 at 18:25
  • A version of Pete Becker's solution can be found in my post: -> http://stackoverflow.com/questions/20406744/ – Adolfo Dec 26 '14 at 15:04

1 Answers1

3

Strings are much smarter than you give them credit for. They know how to search, so you don't have to do it yourself.

int pos = 0;
int match_pos;
std::string result;
while ((match_pos = target.find(oldVal, pos)) != std::string::npos) {
    result += target.substr(pos, match_pos - pos);
    result += newVal;
    pos = match_pos + target.size();
}
result += target.substr(pos, std::string::npos);

Sorry, this is a sketch; not tested, but you get the idea.

Nawaz
  • 353,942
  • 115
  • 666
  • 851
Pete Becker
  • 74,985
  • 8
  • 76
  • 165