Sign in to follow this  
Obs

C++ Removal of char from string

Recommended Posts

Obs    101
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.

[code]
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;
}
[/code]

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 this post


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

Share this post


Link to post
Share on other sites
Wooh    1088
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.
[code]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;
}[/code]

Share this post


Link to post
Share on other sites
Trienco    2555
Quick and not flexible:

[code]
bool filter(char c) { return std::string(" ():,").find(c) != std::string::npos; };

str.erase( remove_if( str.begin(), str.end(), filter ), str.end() );
[/code]

Reusable for any list of characters:
[code]
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() );
[/code]

Share this post


Link to post
Share on other sites
Obs    101
@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 this post


Link to post
Share on other sites
Trienco    2555
[quote name='YogurtEmperor' timestamp='1314001499' post='4852177']
You also never scan the first character of the string.

Try this instead:
[code]for (int it = tempSize; --it >= 0; )[/code]
[/quote]

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.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this