# C++ Removal of char from string

This topic is 2379 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

I was wondering if anyone could shed some light on a problem I'm having.

I have a string that I am removing unwanted characters from. The characters I want removed are:

(
)
:
,
space

The first four characters are no problem, but any conditional statements based upon the space do not fire when tested at the same time as the first four.

E.g. the following doesn't work. All characters apart from space are removed. If I test only for the space, then spaces are removed.

 string removeCharacters(string modified) { int tempSize = modified.size(); for (int it = ( tempSize-1); it > 0; it-- ) { if ( ( modified[it] == ' ' ) || ( modified[it] == '(') ) || ( modified[it] == ')' ) || ( modified[it] == ':' ) || ( modified[it] == ',' ) ) { modified.erase(it, 1); it--; } } return modified; } 

I've tried case statements, too. Same behaviour (understandable). However, if I seperate the space condition from the if statement and put its own case statement, it works. Likewise if I seperate it into its own if statement from a main case statement it works. Seperating into the same kind of statement does not work.

I was originally using string::rfind() and searching for each character to be removed in turn and that worked. I don't know why I started playing around with case/if statements, but this behaviour has been bothering me for a while and I refuse to go back to rfind() without understanding what is wrong here.

##### Share on other sites
The code you posted has a syntax error. You have one extra ) after ( modified[it] == '('). You should also remove it--; because that will make you skip the next char in the string.

##### Share on other sites
First off, it is generaly considered bad practice to modify a loop inside of itself. It tends to produce unpredictable results.

##### Share on other sites
In these cases I think it quite nice to use a switch to test the values. So I made a version of your function using switch just so you can see, I also use iterator instead of an index.
string removeCharacters(string modified) { string::iterator it = modified.begin(); while (it != modified.end()) { switch (*it) { case ' ': case '(': case ')': case ':': case ',': it = modified.erase(it); break; default: ++it; } } return modified; }

##### Share on other sites
Quick and not flexible:

 bool filter(char c) { return std::string(" ():,").find(c) != std::string::npos; }; str.erase( remove_if( str.begin(), str.end(), filter ), str.end() ); 

Reusable for any list of characters:
 struct CharFilter { CharFilter(const std::string& filter) : chars(filter) {} bool operator()(char c) { return chars.find(c) != std::string::npos; } std::string chars; }; str.erase( remove_if( str.begin(), str.end(), CharFilter(" (),:") ), str.end() ); 

##### Share on other sites
@wooh:

The extra ) is not in my code as it wasnt a straight copy/paste, but well spotted. As for skipping the next character with it--, that's my problem right there - so thank you! It's was left over from my rfind() implementation to save an uneccessary iteration as the string is 'shortened' by one and I never even considered if I needed it in this implementation (which uses a different iteration). A space ONLY occurs between two other 'forbidden' characters so it was always skipped.

The switch route was my preferred solution, too. I switched to an if statement when it wasn't working (I had left it-- in that too). It also makes more sense to pass string::iterator to erase, although I'm going to stick with working backwards through the string.

Thank you again for the reply.

Trienco, thank you for the reply, too - I'll take a look at your code, though I wasn't explicitly looking for a new solution.

##### Share on other sites
You also never scan the first character of the string.

for (int it = tempSize; --it >= 0; )

L. Spiro

##### Share on other sites

You also never scan the first character of the string.

for (int it = tempSize; --it >= 0; )

Ieek... My first reaction was "now your first access is invalid". Then I saw the rather disturbing use of side effects in the condition. I might be tempted to kick someone at work for throwing that kind of curve ball just to avoid writing a -1 (and not at least making it a more readable while-loop).

Though generally the bugs pointed out are one more reason to just use the convenient methods of the standard library and avoiding any potential for bugs in the first place.