0

I need to trim the beginning or end of a string, given a matching character.

My function definition looks:

void trim(std::string &s, char c, bool reverse = false);

The bool reverse flags whether to trim the beginning (false) or end (true) of the string.

For example:

s = "--myarg--";
trim(s, '-', false); // should set s to "myarg--"
trim(s, '-', true);  // should set s to "--myarg"

To trim the beginning (i.e. reverse=false), this works fine:

bool ok = true;
auto mayberemove = [&ok](std::string::value_type ch){if (ch != '-') ok = false; return ok;};
s.erase(std::remove_if(s.begin(), s.end(), mayberemove), s.end());

The lambda just returns true for each character matching '-', up until the first occurrence of a non-matching character, and continues to return false thereafter. Here I've hard-coded the matching char to be '-', to make the code easier to read.

The trouble I am having is with trimming in reverse. This doesn't work - same as above, but with reverse iterators and ::base():

s.erase(std::remove_if(s.rbegin(), s.rend(), mayberemove).base(), s.end());

Instead, the line above trims all of the ending characters except for the first two.

Any ideas? Thanks

Blair Fonville
  • 908
  • 1
  • 11
  • 23
  • Shouldn't be too hard for you to plop all that into a [mcve]. Right now you say you are having problems with the reverse, but only offer up one like of the reverse code. – user4581301 Sep 28 '17 at 23:17
  • This answer may be of use (allows left and right trimming) https://stackoverflow.com/questions/216823/whats-the-best-way-to-trim-stdstring/25385766#25385766 – Galik Sep 28 '17 at 23:31
  • 1
    @rici that doesn't work. I don't see that remove has that construct, and if it did, it would remove all "-" characters from the entire string - which isn't what I was after. – Blair Fonville Sep 28 '17 at 23:47
  • @blair: Ok, that's true. What you really want to use is `find_if_not`). – rici Sep 28 '17 at 23:55
  • You should not use a `bool` for the reverse parameter. It make the code harder to understand. Use an `enum class` or have multiple functions like `trim`, `trim_left` and `trim_right` instead. If not yet convinced, then read books on good coding practices. – Phil1970 Sep 28 '17 at 23:55
  • @Phil1970 actually, I'm perfectly convinced, and you're exactly right. I'm trying to get away from bad habits like this - thanks for pointing it out. I'll split it into distinct functions. – Blair Fonville Sep 29 '17 at 00:13

2 Answers2

3
std::string& trim( std::string& s, char c, bool reverse = false )
{
  return reverse
    ? s.erase( s.find_last_not_of( c ) + 1 )
    : s.erase( 0, s.find_first_not_of( c ) );
}
bgfvdu3w
  • 1,548
  • 1
  • 12
  • 17
Dúthomhas
  • 8,200
  • 2
  • 17
  • 39
  • Function is defined to return `void`. – bgfvdu3w Sep 28 '17 at 23:47
  • @Mark actually, apart from the return, that does work. I just tested it with a bunch of "-"s. – Blair Fonville Sep 28 '17 at 23:54
  • I'll probably go with this. I haven't seen those functions before. It seems to be perfectly concise, and I would guess very efficient. – Blair Fonville Sep 28 '17 at 23:58
  • @BlairFonville Aye, I didn't give it a good look at first, it's actually simple and clever. – bgfvdu3w Sep 28 '17 at 23:58
  • Yes, oops, I always use a much more capable version than this, and shoe-horned it into OP's function signature. Thanks for the return type edit Mark! – Dúthomhas Sep 29 '17 at 00:08
  • Wholeheartedly approve of the return of the string. Instantly makes it way more useful, in an initialization, for example. In fact, I'd be tempted to `std::string trim(std::string s, char c, bool reverse = false)` so you don't trash the input string. – user4581301 Sep 29 '17 at 00:12
  • Actually, I do too. I return a reference in my actual implementation. I don't know why I changed it here. – Blair Fonville Sep 29 '17 at 00:18
0

Well, actually I just figured it out. It looks like I needed to reverse the logic of the mayberemove lambda for the reverse case. So, this appears to work fine:

if (reverse) {
    bool notok = false;
    auto mayberemove = [&notok](std::string::value_type ch){if (ch != '-') notok = true; return notok;};
    s.erase(std::remove_if(s.rbegin(), s.rend(), mayberemove).base(), s.end());
}
else {
    bool ok = true;
    auto mayberemove = [&ok](std::string::value_type ch){if (ch != '-') ok = false; return ok;};
    s.erase(std::remove_if(s.begin(), s.end(), mayberemove),s.end());
}

And that works. Now I just need to understand why.

Blair Fonville
  • 908
  • 1
  • 11
  • 23
  • Alternatively, you can do `s.erase(s.begin(), std::remove_if(s.rbegin(), s.rend(), mayberemove).base())` when `reverse = true` – bgfvdu3w Sep 28 '17 at 23:20
  • A good way to better look at what's going on would be to add something like `if (notok) { cout << 'r';} cout << ch;`. Do this right and the same lambda should work for both. I'm thinking something like `[&ok](char ch) { if (ch != '-') { ok = false; } return ok; };` – user4581301 Sep 28 '17 at 23:53
  • That said, this is probably way more work than you need to do as it always scans the whole string even though its job is almost always done fairly early. – user4581301 Sep 28 '17 at 23:54
  • @user4581301 I was think that too. It's not as efficient, but I got stuck just wanted to get it to work for for my own edification. – Blair Fonville Sep 28 '17 at 23:56
  • Pretty much what I figured. Going in the wrong direction you still learn something you can apply later. Often it's, "Well, I won't do that again!" but there are some gems. – user4581301 Sep 29 '17 at 00:08
  • @user4581301 God I had a lot of typos in that comment. Glad you could read through them. – Blair Fonville Sep 29 '17 at 00:28