7
\$\begingroup\$

This function is intended to remove leading and trailing whitespace from a string. How can it be made more efficient? For example, can the two for loops be combined into one?

string trim(string str)
{
    int i = 0;
    for (char c : str)
    {
        if (!isspace(c))
            break;
        i++;
    }

    string trimmed = str.substr(i, (str.length()-i));

    i = 0;
    for (char c : str)
    {
        if (isspace(c))
            break;
        i++;
    }

    trimmed = trimmed.substr(0, i);
    return trimmed;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ For starters, you should pass str by const& since it's not being modified. \$\endgroup\$ – Jamal Jan 27 '14 at 2:05
  • \$\begingroup\$ @Jamal Good advice in general, but I'm on the fence here. I think it may be reasonable to take a non-const non-reference, then modify it (use .erase instead of .substr). Maybe that's because I would have expected the name trim to mean in-place. \$\endgroup\$ – Michael Urman Jan 27 '14 at 2:57
  • \$\begingroup\$ @MichaelUrman: That too. I was adapting that to what was already here. I've noted in my review that it doesn't work, but I've deleted it as my own code snippet removes all whitespace instead of just leading and trailing. \$\endgroup\$ – Jamal Jan 27 '14 at 3:00
  • \$\begingroup\$ You are assuming that the string contains 1 word. If your string contains multiple words then you will loose all but the first. \$\endgroup\$ – Martin York Jan 28 '14 at 21:54
  • 1
    \$\begingroup\$ @lifebalance C++ isn't a very good language. \$\endgroup\$ – Celeritas Jan 6 '15 at 6:07
10
\$\begingroup\$

Your current implementation you will lose all but the first word:

std::string str("this is a sentence");
std::string t  = trim(str);

std::cout << t << "\n" 

// Output
this

If that is your intention then there is an easier way of doing that:

std::string trim(std::string const& str)
{
    std::string word;
    std::stringstream stream(str)
    stream >> word;

    return word;
}

If you want to trim space of the ends on the string, then I would use the string search functions [coliru demo]:

std::string trim(std::string const& str)
{
    if(str.empty())
        return str;

    std::size_t firstScan = str.find_first_not_of(' ');
    std::size_t first     = firstScan == std::string::npos ? str.length() : firstScan;
    std::size_t last      = str.find_last_not_of(' ');
    return str.substr(first, last-first+1);
}

Here is the string documentation. Boost also has some extra libraries on the subject.

| improve this answer | |
\$\endgroup\$
4
\$\begingroup\$

What about boost?

std::string untrimmed( "   This is an untrimmed string!    " );
std::string trimmed( boost::algorithm::trim( s ) );

It doesn't get much terser than that, and I trust the boost libs to be more efficient than what I could hack together on a whim...

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Would be my way to go. boost is always a good idea. \$\endgroup\$ – anhoppe May 9 '14 at 20:02
2
\$\begingroup\$

To make this more efficient, you have to consider what's inefficient about it. I see two main contenders:

  1. Data copies
  2. Data scans

You copy data in up to three places. First when str is passed into the function it may be copied or it may be moved. Second when you take trimmed = str.substr(..) you copy a presumably large portion of the string. Third when you take trimmed = trimmed.substr(..) you do so again. In the worst case (when no whitespace is removed) this is two or three full copies of your string.

You scan data in two places. First when you scan str forward, looking for non-whitespace. Second when you scan str forward, looking for whitespace. The latter appears to be a mistake, as you use the index this calculates against trimmed, and, as Jamal found, strings with leading whitespace result in empty strings being returned. It seems to me that instead of finding the first whitespace character, you need to find the last non-whitespace.

My recommendations are as follows. If run-time efficiency is your ultimate concern (as opposed to programmer efficiency or other concerns), consider changing your function signature to pass a string by reference. This avoids the initial possible copy. Then avoid the other two copies by modifying the passed string in place with std::string::erase instead of std::string::substr. If you stick to std::string::substr, definitely find both ends and only call substr once. Second, unless you mean something unusual by "leading and trailing whitespace" (such as returning the first whitespace delimited substring) scan for trailing whitespace from the end of your string.

If run-time efficiency is your ultimate concern, you can't optimize properly without knowing more about your data. Do you typically see large amounts of whitespace at one or both ends of your string, or is it typically a relatively small amount? If possible, profile a few approaches against a representative data set. If you typically extract small desired strings from within large amounts of whitespace, it's possible that a passing a const string& and returning the results of a single call to std::string::substr(pos, len) will be faster. But if you typically remove no whitespace, it's hard to beat in-place even with C++11's move semantics.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

If readability and maintenance is a concern something like this might help:

const string whitespace = " \t\f\v\n\r";
string test = "    test1 test2 test3     \n\n";
int start = test.find_first_not_of(whitespace);
int end = test.find_last_not_of(whitespace);
test.erase(0,start);
test.erase((end - start) + 1);

This still iterates over the string twice but is much more concise.

One way to only iterate once would be like this:

string test2 = "    test1 test2 test3     \n\n";
int start2 = 0, end2 = test2.length() - 1;
while(isspace(test2[start2]) || isspace(test2[end2]))
{
    if(isspace(test2[start2]))
        start2++;
    if(isspace(test2[end2]))
        end2--;
}
test2.erase(0,start2);
test2.erase((end2 - start2) + 1);

This iterates up to the maximum number of whitespace characters on either end.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Loki Astari's ideas will work (for the most part) except for the last one, in one special (but certainly no less important) case: If the string being split is all spaces.

This can be fixed with a simple conditional statement:

std::string trim(std::string const& str)
{
    std::size_t first = str.find_first_not_of(' ');

    // If there is no non-whitespace character, both first and last will be std::string::npos (-1)
    // There is no point in checking both, since if either doesn't work, the
    // other won't work, either.
    if(first == std::string::npos)
        return "";

    std::size_t last  = str.find_last_not_of(' ');

    return str.substr(first, last-first+1);
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ It would be more clear and maintainable if you changed the -1 to std::string::npos. \$\endgroup\$ – jliv902 May 9 '14 at 18:31
  • \$\begingroup\$ This is a good point. I'll edit it. \$\endgroup\$ – Gurgadurgen May 9 '14 at 19:43
0
\$\begingroup\$

From various source and after a lot experiments, the code below can be used in the following manner:

RemoveLTWhiteSp(Inpu_strng, &res_strng);

I always use this code and it works fine as a trim function. It removes leading and trailing white spaces.

void RemoveLTWhiteSp(std::string &InStr_Token, std::string *OutStr)
{
    if( 0 != InStr_Token.size() )   //if the size is 0
    {
        std::string wspc (" \t\f\v\n\r");// These are the whitespaces
        //finding the last valid character        
        std::string::size_type posafter = InStr_Token.find_last_not_of(wspc);
        //finding the first valid character
        std::string::size_type posbefore=InStr_Token.find_first_not_of(wspc);

        if((-1 < (int)posafter) && (-1 < (int)posbefore)) //Just Wsp
        {
            std::string NoSpaceToken;
            // Cut off the outside parts of found positions
            NoSpaceToken = InStr_Token.substr(posbefore,((posafter+1)-posbefore));
            *OutStr = NoSpaceToken;
        }
    }
}

Here is a usage example with a very efficient tokenizer using a vector. The tokens argument is just a "vector string":

template<class ContainerT>

void tokenize(const std::string& str, ContainerT& tokens, const std::string& delimiters , bool trimEmpty = false )
{
    std::string::size_type pos, lastPos = 0;
    std::string Instr;
    std::string OutStr;

    while(true)
    {
        pos = str.find_first_of(delimiters, lastPos);
        if(pos == std::string::npos)
        {
            pos = str.length();

            if(pos != lastPos || !trimEmpty)
            {
                Instr = std::string(ContainerT::value_type(str.data()+lastPos, (ContainerT::value_type::size_type)pos-lastPos ));
                RemoveLTWhiteSp(Instr, &OutStr);
                tokens.push_back(OutStr);
            }
            break;
        }
        else
        {
            if(pos != lastPos || !trimEmpty)
            {
                Instr = std::string(ContainerT::value_type(str.data()+lastPos,  (ContainerT::value_type::size_type)pos-lastPos ));
                RemoveLTWhiteSp(Instr, &OutStr);
                tokens.push_back(OutStr);
            }
        }
        lastPos = pos + 1;
    }
};
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.