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

Simple string joiner in modern C++

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

Problem

(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 
#include 
#include 
#include 
#include 
#include 

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

template
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 
std::string join(T begin, T end, std::string separator)
{
    return join(begin, end, separator, "");
}

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

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

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


Any idea how to improve this?

Solution

Your code is fine, except for three small quirks

  • You don't need overloads if you provide default arguments



  • You don't need constant Ts



  • You don't need to check the length of your collection (except for begin == end).



template
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) 
(1 element  case)  
(2 elements case)    
(3 elements case)      


As you can see, except for the first element, you will always get 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 ``:

template
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::value_type;

    std::copy(begin, end, std::ostream_iterator(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.

Code Snippets

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();
}
(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>
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();
}

Context

StackExchange Code Review Q#142902, answer score: 9

Revisions (0)

No revisions yet.