Simple string joiner in modern C++

Question

(See the next iteration.)

I have this small template function for conveniently dumping a sequence into a string such that there is a delimiter between two consecutive elements, and no such after the last element:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <sstream>
#include <string>
#include <vector>

using std::cout;
using std::endl;
using std::for_each;
using std::vector;

template<typename T>
std::string join(const T begin,
                 const T end,
                 std::string separator,
                 std::string concluder)
{
    const auto length = std::distance(begin, end);
    std::stringstream ss;
    size_t count = 0;

    for (T iter = begin; iter != end; ++iter, ++count)
    {
        ss << *iter;

        if (count < length - 1)
        {
            ss << separator;
        }
    }

    ss << concluder;
    return ss.str();
}

template<typename T>
std::string join(T begin, T end, std::string separator)
{
    return join(begin, end, separator, "");
}

template<typename T>
std::string join(T begin, T end)
{
    return join(begin, end, ", ");
}

int main() {
    vector<vector<int>> matrix = {
        { 1, 2, 3 },
        { 4, 5 },
        { },
        { 10, 26, 29 }
    };

    for_each(matrix.cbegin(),
             matrix.cend(),
             [](std::vector<int> a) {
                 cout << join(a.cbegin(), a.cend()) << endl;
             });
}

Any idea how to improve this?

Answer 1

I assume that you probably want to users to be able to pass ForwardIterator InputIterator. I will explain why I made the assumption.

According to assumption, you can rename the T into InputIt. Iterators denoting range are usually called first and last, even though last is not actually last.

template<typename InputIt>
std::string join(InputIt first,
                 InputIt last,
                 std::string separator,
                 std::string concluder)

I've never seen iterators being passed as const. Probably you wanted iterators referring to const objects? It is common to say const iterator, but it doesn't mean that iterators are const.

count and length variables are somewhat odd. The pair of iterators should denote the range needed, and it is work of the operator!=() to check if the end is hit.

for (T iter = begin; iter != end; ++iter, ++count)

Never seen traversing the range like that. People usually use this:

while (first != last)
{
    ss << *iter++;
    ss << separator;
}

It will need to be tweaked a bit to not output the separator at the end. I'll use what @Zeta suggested (awesome idea):

We'll check if the range is empty, then print the first element:

if (first == last)
{
    return concluder;
}

std::stringstream ss;
ss << *first;
++first;

And then we swap sequence of output in the loop, so we output separator first, then the element. This way, there won't be separator at the end:

while (first != last)
{
    ss << separator;
    ss << *first;
    ++first;
}

Even though preincrement is slightly faster than postincrement, my benchmarking program didn't show significant difference (it is less than 1%). Nevertheless, I've used preincrement to not make life harder for custom iterators.

Then we simply output concluder:

ss << concluder;

Rather than creating multiple overloads use default arguments:

template <typename InputIt>
std::string join(InputIt first, InputIt last, const std::string& separator = ", ", const std::string& concluder = "")

And const correctness, of course.

Full code:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <sstream>
#include <string>
#include <vector>

using std::cout;
using std::endl;
using std::for_each;
using std::vector;

template<typename InputIt>
std::string join(InputIt first,
    InputIt last,
    const std::string& separator = ", ",
    const std::string& concluder = "")
{
    if (first == last)
    {
        return concluder;
    }

    std::stringstream ss;
    ss << *first;
    ++first;

    while (first != last)
    {
        ss << separator;
        ss << *first;
        ++first;
    }

    ss << concluder;

    return ss.str();
}

int main() {
    vector<vector<int>> matrix = {
        { 1, 2, 3 },
        { 4, 5 },
        {},
        { 10, 26, 29 }
    };

    for_each(matrix.cbegin(),
        matrix.cend(),
        [](std::vector<int> a) {
        cout << join(a.cbegin(), a.cend()) << endl;
    });
}

I don't really like using, but I don't think it is of any importance here.

Using input itertors will yield basic exception safety, since the range is invalid after it being passed once. For other supported iterator types the function has strong exception safety guarantee.

Answer 2

Your code is fine, except for three small quirks

1. You don't need overloads if you provide default arguments
2. You don't need constant Ts
3. You don't need to check the length of your collection (except for begin == end).

template<typename InputIt>
std::string join(InputIt begin,
                 InputIt end,
                 const std::string & separator =", ",  // see 1.
                 const std::string & concluder ="")    // see 1.
{
    std::ostringstream ss;

    if(begin != end)
    {
        ss << *begin++; // see 3.
    }    

    while(begin != end) // see 3.
    {
        ss << separator;
        ss << *begin++;
    }

    ss << concluder;
    return ss.str();
}

You only need to check whether you have an empty range. If you don't, you start with the first one, and then stream the separator before the next element:

(0 elements case) <concluder>
(1 element  case) <element> <concluder>
(2 elements case) <element> <separator> <element> <concluder>
(3 elements case) <element> <separator> <element> <separator> <element> <concluder>

As you can see, except for the first element, you will always get <separator> <element> for any other element. This adds a little bit more logic to the empty case, but you don't need to traverse your range twice, and this method makes your algorithm eligible for input iterators.

Other than that, the only other thing I would change is to use an ostringstream instead of an stringstream. This prevents you from accidentally using >>. And global using std::*** isn't my cup of tee either. You can, however, move that into your main.

But here's a little exercise: you can just use an <iterator>:

template<typename InputIt>
std::string join(InputIt begin,
                 InputIt end,
                 const std::string & separator =", ",  // see 1.
                 const std::string & concluder ="")    // see 1.
{
    std::ostringstream ss;

    using value_type = typename std::iterator_traits<InputIt>::value_type;

    std::copy(begin, end, std::ostream_iterator<value_type>(ss, separator.c_str()));

    ss << concluder;
    return ss.str();
}

However, as you've noted, it does not yield the same result as your variant. Here's the exercise, though: write an iterator that provides your behavior.

Answer 3

I have always used a separator variable that I assign to after first use.

    template<typename InputIt>
    std::string join(InputIt first, InputIt last,
        const std::string& separator = ", ",
        const std::string & concluder = "")
    {
        const std::string empty {};
        auto sep = &empty;
    
        std::stringstream ss;
    
        while (first != last)
        {
            ss << *sep << *first++;
            sep = &separator;
        }
    
        ss << concluder;
    
        return ss.str();
    }

Whether you use a pointer (as here) or a simple copy depends on preference, I suppose. For performance, well, you pay an extra assignment at every iteration, and there's the stringstream to set up even for empty containers. The thing is, it's very simple!