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

Generic container assignment

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

Problem

For setters that set a member collection, I write template functions so as to accept any collection type as input:

class HasItems
{
public:
    template 
    void items(const C& items) {
        // Copy-and-swap omitted
        m_items.assign(std::begin(items), std::end(items));
    }

private:
    std::vector m_items;
}


But suppose Item is big, and I want to move them around instead of copy if possible. And furthermore, suppose someone passed me an xvalue std::vector; I could just move that whole container into place and avoid touching the elements at all! That looks like this:

class HasItems
{
public:
    template 
    void items(C&& items) {
        // Helper function implemented below
        assign(m_items, std::forward(items));
    }

private:
    std::vector m_items;
}

// Assigns the contents of src to dest, moving as much as possible
template 
void assign(C1& dest, C2&& src) {
    assign(dest, std::forward(src), std::is_assignable{});
}

// Called if we can use operator=
template 
void assign(C1& dest, C2&& src, std::true_type use_equals) {
    dest = std::forward(src);
}

// Called if we must use dest.assign()
template 
void assign(C1& dest, C2&& src, std::false_type use_equals) {
    assign_elements(dest, std::forward(src), std::is_rvalue_reference{});
}

// Called if we can move the elements
template 
void assign_elements(C1& dest, C2&& src, std::true_type move) {
    dest.assign(std::make_move_iterator(std::begin(src)),
                std::make_move_iterator(std::end(src)));
}

// Called if we must copy the elements
template 
void assign_elements(C1& dest, C2&& src, std::false_type move) {
    dest.assign(std::begin(src), std::end(src));
}


Is this a good approach? In particular, is there a nice way to avoid the tag dispatch on std::is_rvalue_reference in the assign_elements() case? (It was avoided in the operator= case by using dest = std::forward(src) instead of using std::move() conditionally.) It seems to pass

Solution

I've implemented this idiom and discovered a major caveat after a few weeks: it doesn't play nicely with boost::iterator_range or other collection views. An example:

std::vector dest;
std::list src{"foo", "bar"};
// Moves elements out of src, expected
assign(dest, std::move(src));

src = {"foo", "bar"};
// Also moves elements out of src!!!
assign(dest, boost::make_iterator_range(src));


This happens because boost::make_iterator_range(src) is an xvalue. I don't see a good way to distinguish between "container" (meaning safe to move from xvalues) and "container view" (meaning not safe to move from).

Code Snippets

std::vector<std::string> dest;
std::list<std::string> src{"foo", "bar"};
// Moves elements out of src, expected
assign(dest, std::move(src));

src = {"foo", "bar"};
// Also moves elements out of src!!!
assign(dest, boost::make_iterator_range(src));

Context

StackExchange Code Review Q#83443, answer score: 4

Revisions (0)

No revisions yet.