11

I'm implementing a factory class which builds a vector of uint8_t. I want to be able to utilize move semantics when returning the resulting vector. This seems to work but I'm not confident this is the correct way of accomplishing what I want.

I have seen quite a few examples of how a returned automatic variable will be regarded as an rvalue and use the move constructor of the calling code, but in my example, the returned object is a member. I know the member will loose its contents if the caller puts the return value into a move constructor - and this is just what I want.

I have written it something like this:

#include <cstdint>
#include <iostream>
#include <vector>

class Factory
{
public:
    std::vector<uint8_t> _data;

    Factory(std::size_t size) :
        _data(size, 0)
    {
    }

    void buildContent(int param)
    {
        // perform operations on the contents of _data
    }

    std::vector<uint8_t> && data()
    {
        return std::move(_data);
    }
};

int main()
{
    Factory factory(42);
    factory.buildContent(1);
    std::vector<uint8_t> temp(factory.data());
    std::cout << "temp has " << temp.size() << " elements" << std::endl;
    std::cout << "factory._data has " << factory._data.size() << " elements" << std::endl;

    return 0;
}

Edit:

Oh, and the example code outputs the following:

temp has 42 elements
factory._data has 0 elements
anorm
  • 2,255
  • 1
  • 19
  • 38
  • 1
    So what's your question? – DanielKO Oct 02 '13 at 09:31
  • "I'm not confident this is the correct way of accomplishing what I want." – Jaffa Oct 02 '13 at 09:35
  • Oh, I see now. Check this one out: http://stackoverflow.com/questions/9914548/using-of-rvalue-references-in-c11 – DanielKO Oct 02 '13 at 09:41
  • This is directly related to your question: http://stackoverflow.com/questions/5770253/is-there-any-case-where-a-return-of-a-rvalue-reference-is-useful – DanielKO Oct 02 '13 at 09:46
  • I would put the word `move` in your `data` method name. As in `factory.move_data()`. In addition, returning a `std::vector` instance instead of a `&&` might be worthwhile, as you'd get better lifetime behavior at little if any runtime cost. – Yakk - Adam Nevraumont Oct 02 '13 at 15:27

3 Answers3

8

First of all, you have to decide what you want. Do you really want to suck data out of your instance? If so, what you have achieves that fine. On the other hand, it looks quite unsafe. What you could do is use reference qualifiers to enforce that you only return an rvalue reference when the instance itself is an rvalue reference:

std::vector<uint8_t> && data() && // called on rvalue
{
    return std::move(_data);
}

// return lvalue ref or value
std::vector<uint8_t>& data() & // called on lvalue
{
    return _data;
}

In the end, it all depends on which problem you are trying to solve, which is not apparent from your question.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
2

If your compiler has rvalue references to this (&& after methods), you might want to use that. See @juanchopanza's answer.

If you don't, you first want to make sure that data() makes it clear that you are moving. There are a few ways to do this.

First, non-member methods (friends) can override on &&. So you can get this syntax:

std::vector<uint8_t> temp(get_data( std::move(factory) );

where get_data has && and & overloads on your factory type, and either moves or not based off it.

Next, you want to return a std::vector<uint8_t> instead of a `std::vector<uint8_t>&& for lifetime extension issues. The runtime cost is somewhere between zero and small, but the bugs eliminated are worthwhile.

If create_factory returns a Factory object, then if we do this:

for( uint8_t x : get_data( create_factory() ) )

the get_data that returns a && doesn't work, while the one that returns a temporary works flawlessly.

What is going on? Well, ranged-based for loops are defined as binding the thing you are iterating over to a reference. A temporary bound to a reference has its lifetime extended: a reference bound to a reference has no lifetime extension. The create_factory function's return value's lifetime is not extended in either case.

With the && case, the reference to vector is left dangling. With the return-temporary case, the factory's vector is moved into the temporary, then that temporaries lifetime is extended.

In short, returning && references is very rarely a good idea.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
0

How about hiding the assignment operator in your vector and only implementing move assignment operator (essentially disallowing copying - if that's what you want).

So that this:

std::vector<uint8_t> temp(factory.data());

will not compile unless you change it to this :

std::vector<uint8_t> temp(std::move(factory));
Sereger
  • 1,083
  • 1
  • 9
  • 11
  • How would I go about hiding std::vector's copy constructor or assignment operator? – anorm Oct 08 '13 at 11:18
  • What I meant to say is if you change the meaning of your factory to act more like an extension to a vector by either inheriting (IS-A vector) or wrapping it (HAS-A vector) your clients would then deal with a factory (which you would have to rename to reflect the new meaning). Alternatively you could create a non-copyable vector and return it from factory instead of std::vector but that might sound too complicated. The problem with your approach is the call to facotry.data() will invalidate that data which might be confusing. – Sereger Oct 09 '13 at 16:09