1

I want to write a method that should trim a std::string's ongoing and trailing white spaces. For example, if the method gets " HELLO WORLD ", it should return "HELLO WORLD". Note, that between the return string is a white space, that is important.

This is the signature of my method:

std::string MyHandler::Trim(const std::string& toTrim)

My approach was to copy the 'toTrim' parameter into a non constant copy.

std::string toTrimCopy = toTrim;

Now I want to get a non constant iterator and erase within a for loop any whitespaces from beginning and the end, while the iterators value is a white space.

for (std::string::iterator it = toTrim.begin(); *it == ' '; it++)
{
    toTrimCopy.erase(it);
}

for (std::string::iterator it = toTrim.end();
     *it == ' ';
     it--)
{
    toTrimCopy.erase(it);
}

This results in the compiler error:

StringHandling.C:60:49: error: conversion from ‘std::basic_string<char>::const_iterator {aka __gnu_cxx::__normal_iterator<const char*, std::basic_string<char> >}’ to non-scalar type ‘std::basic_string<char>::iterator {aka __gnu_cxx::__normal_iterator<char*, std::basic_string<char> >}’ requested

I come from Java and I learn C++ for 3 weeks now. So dont judge me. I suspect, that the = assignment assigns constant char pointer to my new string, so that my copy is implicit a constant value. But I do not exactly know.

By the way, this approach throws an exception as well:

    std::string toTrimCopy = "";
    std::strcpy(toTrimCopy, toTrim);

He says, that he cant convert a string to a char pointer.

Werner Henze
  • 16,404
  • 12
  • 44
  • 69
  • 6
    An iterator is specific to the container (or string) it was created from. You can't use iterators from one container (or string) to reference elements in another container (or string). – Some programmer dude Oct 21 '20 at 08:23
  • 1
    Also, the `end` iterator can't be dereferenced, that leads to *undefined behavior*. – Some programmer dude Oct 21 '20 at 08:23
  • 1
    Finally please don't try to reinvent the wheel: See [What's the best way to trim std::string?](https://stackoverflow.com/q/216823/440558) – Some programmer dude Oct 21 '20 at 08:24
  • Your suspicions are wrong. `=` does the simple thing, no connection remains between the copied from and copied to strings. Your error is elsewhere. – john Oct 21 '20 at 08:25
  • Since you are trying to use an iterator from one string on another, you could just rewrite your code to use integers as indexes instead. Although as said above, you're not writing the most efficient code. – john Oct 21 '20 at 08:29
  • 4
    *I come from Java and I learn C++ for 3 weeks now* -- Don't use Java as a model in writing C++ code. C++ is value-based, not reference-based like Java. When you issue a `str1 = str2;` in C++, it does not do the reference shenanigans that Java does -- you actually get the object copied over, just like if you used simple `int` variables. – PaulMcKenzie Oct 21 '20 at 08:32
  • Pet peeve: putting functions in a class that have no place there. C++ has free functions – Caleth Oct 21 '20 at 08:54

2 Answers2

1

It's undefined behaviour to pass an iterator from toTrim to methods of toTrimCopy, so you are lucky that the type mismatch caught it.

std::string::begin has two overloads:

 std::string::iterator std::string::begin();
 std::string::const_iterator std::string::begin() const;

The constness of the string participates in overload resolution. You can't initialise an iterator from a const_iterator, because that would allow you to modify the underlying object through that iterator.

I would change your function to

namespace MyHandlerNS { // optional, could be in global namespace
    std::string Trim(std::string toTrim) {
        for (auto it = toTrim.begin(); it != toTrim.end() && *it == ' ';) {
            it = toTrim.erase(it);
        }

        for (auto it = toTrim.rbegin(); it != toTrim.rend() && *it == ' ';) {
            it = toTrim.erase(it.base());
        }

        return toTrim;
    }
}

Or get rid of the loops entirely with standard algorithms

#include <algorithm>

namespace MyHandlerNS { // optional, could be in global namespace
    std::string Trim(std::string toTrim) {
        auto isSpace = [](char c){ return c == ' '; };

        auto it = std::find_if_not(toTrim.begin(), toTrim.end(), isSpace);
        toTrim.erase(toTrim.begin(), it);

        it = std::find_if_not(toTrim.rbegin(), toTrim.rend(), isSpace).base();
        toTrim.erase(it, toTrim.end());

        return toTrim;
    }
}
Caleth
  • 52,200
  • 2
  • 44
  • 75
1

I finally decided me for using regex:

std::string StringHandling::Trim (const std::string& toTrim)
{
    return std::regex_replace(toTrim, std::regex("^ +| +$"), "");
}

Thread could be closed here.