C++ String manipulation isn't making sense to me...

Question

This particular assignment has to do with removing substrings from strings; I am trying some of the Stanford SEE courses online to learn some new languages.

What I've got so far is below, but if text = "hello hello" and remove ="el", it gets stuck in a loop, but if i change text to text = "hello hllo", it works, making me think I'm doing something obviously stupid.

There is a stipulation in the assignment not to modify the incoming strings, and instead to return a new string.

string CensorString1(string text, string remove){
    string returned;
    size_t found=0, lastfound=0;
    found = (text.substr(lastfound,text.size())).find(remove);
    while (string::npos != found ){
        returned += text.substr(lastfound,found);
        lastfound = found + remove.size();
        found = (text.substr(lastfound,text.size())).find(remove);
    }
    returned += text.substr(lastfound,found);
    return returned;
}

Guidance would be appreciated :-) Thanks

UPDATE

Took the very kind advice given and modified my code to this :

string CensorString1(string text, string remove){
string returned;
size_t found=0, lastfound=0;
found = text.find(remove);
while (string::npos != found ){
    returned += text.substr(lastfound,found);
    lastfound = found + remove.length();
    found = text.find(remove,lastfound);
}
returned += text.substr(lastfound);
return returned;
}

But still behaves the same

Any more ideas folks?

Answer 1

text.substr(lastfound,text.size()) begins somewhere in the middle of text and continues for the entire size of text. That doesn't make sense, although I suppose it works because of the wonky way these particular functions handle range errors. string::npos would state your intention better than size.

Generally speaking, the manipulation members of string are less elegant than the more general algorithms in <algorithm>. I recommend you use those instead, such as std::search, along with string::iterator in place of integer offsets.

UPDATE: The same kind of error exists in the second example. Use string::npos for the second argument to substr to get the substring through the end.

Answer 2

found = (text.substr(lastfound,text.size())).find(remove); is incorrect. It returns the index of the searched string in text.substr(lastfound,text.size()), but not in text.

You should perhaps change this to found = text.find(text, lastfound);.

Besides of being incorrect, taking a substring (this means, allocating a new string) and calculating an index in it is quite inefficient, unless the optimizer is super-smart.

Moreover, the final returned += text.substr(lastfound,found); is incorrect too: you need to add the last chunk of the text, not the one till found index (which is most likely empty, as lastfound can be smaller than found. Better would be returned += text.substr(lastfound);

Edit:
In the second example, you need to replace returned += text.substr(lastfound,found); with returned += text.substr(lastfound,found-lastfound);. The second argument to substr is length, not position.

With this change, the test example runs fine in my test program.

(Addition by J.F. Sebastian:)

string CensorString1(string const& text, string const& remove){
  string returned;
  size_t found = string::npos, lastfound = 0;
  do {
    found = text.find(remove, lastfound);
    returned += text.substr(lastfound, found-lastfound);
    lastfound = found + remove.size();
  } while(found != string::npos);
  return returned;
}

Answer 3

found = (text.substr(lastfound,text.size())).find(remove); starts counting for the find() function all over again at zero (i.e. the first character after the last match will have index 0, and it will look like the start of text). However, find can take 2 parameters, where the second parameter is the index to start at. So you can change this line to found = text.find(remove,lastfound)

More clearly:

text =                      hellohello
find "el" in here            ^ is at index 1 in `text`
substring after match          lohello
find "el" in here                 ^ is at index 3 in the substring
                                    but your program doesn't know
                                    that's actually index 6 in `text`