Why my trim function doesn’t work?

[Ravi]

Refer to this code -->

http://pastebin.com/2w1A8Ti0

I want to trim a string of spaces. First I trip spaces from the
starting and it works fine and then I find the positions of spaces
from the end and try to remove it and it throws exception.

Why?

 

[Francesco S. Carta]

Refer to this code -->

http://pastebin.com/2w1A8Ti0

I want to trim a string of spaces. First I trip spaces from the
starting and it works fine and then I find the positions of spaces
from the end and try to remove it and it throws exception.

Why?

Post your code in the body of your message, several people simply do not
click on such links.

If you make a search on the group's archive you will also find several
working examples of a trimming function, although you could very well
implement your own for learning purposes and ask for help if you get stuck.

But you need to post the code here in first place, for that.

 

[Stefan van Kessel]

Refer to this code -->

http://pastebin.com/2w1A8Ti0

I want to trim a string of spaces. First I trip spaces from the
starting and it works fine and then I find the positions of spaces
from the end and try to remove it and it throws exception.

Why?

"References, pointers, and iterators referring to the elements of a
basic_string sequence may be invalidated by the following uses of that
basic_string object:
[...]
- Calling non-const member functions, except operator[](), at(),
begin(), rbegin(), end(), and rend()." (21.3/5)

from the pastebin (for future reference: I think you should probably
include snippets of that size in your mail):

string::iterator it = str.begin();
string::iterator end = str.end() - 1;

for(; it != str.end() && isspace(*it); it++);
str.replace(str.begin(), it, "");

// +++ I think you already have undefined behavior here.
for(; end >= str.begin() && isspace(*end); end--);

// +++ I think the arguments should be the other way around
str.replace(str.end(), end, "");

 

[Daniel Pitts]

Your code:

void trim(string&str)
{
string::iterator it = str.begin();
string::iterator end = str.end() - 1;

// trim at the starting
for(; it != str.end()&& isspace(*it); it++)
;
str.replace(str.begin(), it, "");

// trim at the end
for(; end>= str.begin()&& isspace(*end); end--)
;
str.replace(str.end(), end, ""); // i get the out_of_range exception
here
}

Your problem is that you assign an iterator into str to 'end' and then
perform a replace on the str which invalidates the iterator that 'end'
contains. The obvious fix is to move the definition of end to after the
first replace.

However, instead of using manual loops without bodies like you did,
maybe you would consider using the standard library for a more elegant
solution?

bool notSpace(string::value_type c)
{
return !isspace(c);
}


void trim(string& str)
{
str.erase(str.begin(), find_if(str.begin(), str.end(),&notSpace));
str.erase(find_if(str.rbegin(), str.rend(),&notSpace).base(),
str.end());
}

The above is a standard variation of the erase-remove idiom.

Wouldn't it be more efficient to remove the end first? Fewer items to copy.

Also, it doesn't seem very exception safe, though I'm not sure where an
exception could occur, doesn't hurt to be safe.

Anything wrong with the following?
void trim(std::string &str)
{
std::string tmp(find_if(str.begin(), str.end(),&notSpace),
find_if(str.rbegin(), str.rend(),&notSpace).base());
tmp.swap(str);
}

 

[Stefan van Kessel]

Anything wrong with the following?
void trim(std::string &str)
{
std::string tmp(find_if(str.begin(), str.end(),&notSpace),
find_if(str.rbegin(), str.rend(),&notSpace).base());
tmp.swap(str);
}

Yes, actually there is. If the string consists only of spaces, it ends
up construction std::string tmp(str.end(), str.rend()) and it will most
likely start reading memory it shouldn't.

Constructing the result in a temporary so that if it something does
throw, the state of the argument is unchanged is a good idea though.

 

[James Kanze]

On 8/22/2010 9:38 PM, Daniel Pitts wrote:

Yes, actually there is. If the string consists only of spaces, it ends
up construction std::string tmp(str.end(), str.rend()) and it will most
likely start reading memory it shouldn't.

Yes. I remember having looked at this problem in the past. You
need to save the intermediate result, in order to use it as the
end criterion for the second search, e.g.:

struct is_space_p : std::unary_function<char, bool>
{
bool operator()( char ch ) const
{
return isspace( static_cast<unsigned char>( ch ) ) != 0;
}
};

std::string
trim( std::string const& original )
{
std::string::const_iterator end
= std::find_if(original.rbegin(), original.rend(),
std::not1(is_space_p())).base();
return std::string(
std::find_if(original.begin(), end, std::not1(is_space_p())),
end);
}

(Of course, this still fails if the original is encoded in
UTF-8.)

Constructing the result in a temporary so that if it something
does throw, the state of the argument is unchanged is a good
idea though.

Using an inout argument to begin with probably isn't a good
idea. Better to return the string as above. (Alternatively,
you can overload, providing a second version:

void trim( std::string* text )
{
*text = trim( *text );
}

But the form returning the string should be the primary
version.)