HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

String Trimmer and trimmed Input routine

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
routineinputtrimmerandtrimmedstring

Problem

As I thought about this question on SO, I realized that I wanted to implement either a string trimmer or a trimmed string inputter.

This is what I came up with, but I have a nagging feeling I could have done better.

Would you critique this, please?

#include 
#include 
#include 
#include 

template 
void
trim(std::basic_string& str, const std::locale& loc)
{
  // (b,e)=ltrim(str)
  typename std::basic_string::const_iterator b = str.begin();
  typename std::basic_string::const_iterator e = str.end();
  while( b != e && std::isspace(*b, loc) )
    b++;

  // (b,e)=rtrim(b,e)
  // Query: this loop looks u-g-l-y. Can it be less ugly?
  while( b != e ) {
    e--;
    if(!std::isspace(*e, loc)) {
      e++;
      break;
    }
  }

  // str=(b,e)
  // Query: this string self-assigns (sort of). Is this UB?
  str.assign(b, e);
}

template 
std::basic_istream&
getTrimmedLine(std::basic_istream& is,
  std::basic_string& str)
{

  // Get raw data from input
  std::getline(is, str);

  // And trim it
  trim(str, is.getloc());

  return is;
}

#define TEST
#ifdef TEST
template
std::size_t countof( T (& a)[N] ) { return N; }

int main () {
  const char * const inputs[] = {
    "simple", " front", "back ", " both ",
    "  front2", "back2  ", "  both2  ",
    "\n", "", "\n\n\n",
    "  \n", "  ", "  \n\n\n"
  };
  const char * const outputs[] = {
    "simple", "front", "back", "both",
    "front2", "back2", "both2",
    "", "", "",
    "", "", ""
  };

  assert(countof(inputs) == countof(outputs));

  for(std::size_t i = 0; i < countof(inputs); i++) {
    std::istringstream is(inputs[i]);
    std::string result;
    getTrimmedLine(is, result);
    assert(result == outputs[i]);
  }
}
#endif

Solution

typename std::basic_string::const_iterator e = str.end();
...

// Query: this loop looks u-g-l-y. Can it be less ugly?
  while( b != e ) {
    e--;
    if(!std::isspace(*e, loc)) {
      e++;
      break;
    }
  }


This looks weird and tends to go UB.

1) e = str.end();

2) e--; / it's better to use --e in this case / you move one more far position after end()!!!

3) std::isspace(*e, loc) <--- UB, accessing the memory that haven't been allocated

If you need to find the first and the last spaces in the string, use this:

b = find_if(str.begin(),  str.end(),  isspace_predicate);
e = find_if(str.rbegin(), str.rend(), isspace_predicate);

Code Snippets

typename std::basic_string<Ch, Tr, Alloc>::const_iterator e = str.end();
...

// Query: this loop looks u-g-l-y. Can it be less ugly?
  while( b != e ) {
    e--;
    if(!std::isspace(*e, loc)) {
      e++;
      break;
    }
  }
b = find_if(str.begin(),  str.end(),  isspace_predicate);
e = find_if(str.rbegin(), str.rend(), isspace_predicate);

Context

StackExchange Code Review Q#9610, answer score: 2

Revisions (0)

No revisions yet.