C++ Copying std::strings from STL container to another STL string

Viewed 154

An ugly block of code that we would like to get rid of

I have this block of code in one of my source files - yuck!

std::string error;
std::vector<string> errors;
// ... populate errors with some strings
for(const auto &item : errors)
{
    if(!error.empty())
    {
        error += std::string("\n");
    }
    error += item;
}

I think there should be a better way to do it, possibly taking inspiration from ostream_iterator. See here

An initial attempt, which on reflection isn't an improvement

My first thought was that perhaps std::for_each would work with a lambda.

std::for_each(errors.cbegin(), errors.cend(),
    [&error] (const auto &item)
    {
        if(!error.empty())
        {
            error += std::string("\n");
        }
        error += item;
    }
);

But then I realized that this is really no improvement what so ever, and in some ways obfuscates the code further through the unnecessary use of a lamba where the range based for loop worked perfectly fine, and was clear to understand.

A similar working example with ostream_iterator

I think that there should be a way to take inspiration from std::copy or std::for_each to do this.

The example given using cout and ostream_iterator is the following:

std::ostream_iterator<int> out_it(std::cout, ", ");
std::copy(myvector.begin(), myvector.end(), out_it);

It is pretty close to what I want to achieve. Instead of std::cout it should use an instance of a std::string object. Instead of ", " the delimiter should be "\n". It already iterates over a vector, and calls out_it, the argument of which should be a string instead of int.

But I don't think this will quite work, since a string does not inherit from ostream. It is not an ostream type.

A final failed attempt

I also attempted this, but didn't really expect it to work. (It doesn't compile.)

std::for_each(errors.cbegin(), errors.cend(),
    std::back_inserter(error)
);

What to do?

3 Answers

I don't think there is too much you can do with this except wrap it in a function so the rest of your code is a little tidier. I went for this version:

std::string smush(std::vector<std::string> const& errors)
{
    std::string error;

    for(auto const& e: errors)
            error += e + '\n';
    error.pop_back();

    return error;
}

Here's a semi-ugly piece of templated code that I use whenever I need to join strings together (I have it in a utilities header), along with an optional end piece too. Will work with any collection type that supports std::begin and std::end (fixed-size arrays as well, though if you have a pointer and length, you'll have to wrap it in a std::span first), and as long as the element inside supports the << operator to make a string with, should work with custom types too.

// Joins collections of strings together.
template<typename InputCollection>
std::string joiner(const InputCollection& collection, const std::string& separator = ", ", const std::string& ending = "")
{
    std::ostringstream os;
    auto begin = std::begin(collection);
    auto end = std::end(collection);

    if (begin != end)
    {
        os << *begin++;
    }

    while (begin != end)
    {
        os << separator;
        os << *begin++;
    }

    os << ending;
    return os.str();
}

So it's wordy, but it's easy-to-understand, which I consider more important. Use like this:

vector<string> collection = { "hey", "there", "all" };
auto output = joiner(collection); // output: "hey, there, all"
std::string stringArray[3] = { "now", "an", "array" };
output = joiner(stringArray, "-"); // Different separator - output: "now-an-array"

So use that if you think it'll be beneficial long-term, but that's obviously longer than what you had originally.

Related