CODEWITHSUNDEEP
c++c++11rvalue-reference
zer0hedge
zer0hedge

Reputation: 479

Is it reasonable approach when C++ function does not 'move from' rvalue reference argument?

Suppose the class Object satisfies the requirements of MoveConstructible and MoveAssignable, but not the requirements of either CopyConstructible or CopyAssignable (i.e. PIMPL with unique_ptr within). Then I have two consumers:

 bool consumer1(Object && arg) {
       if(arg ...) { // arg satisfies some condition
          Object b {std::move(arg)};
          // some processing
          return true;
        }
        return false;
 }

 bool consumer2(Object &&arg) {};

 int main() {
     Object arg;
     // some arg initialization here
     if(!consumer1(std::move(arg))) 
        consumer2(std::move(arg));
 }

i.e. I want to move an instance of Object to consumer1() and if it does not accept it for some reason I move the instance to consumer2().

Alternatively, I could check the state of the instance (i.e. whether it was move from or not) instead of relying of the return value.

While I know that

std::move doesn’t move anything.

I nevertheless feel uncomfortable due to two moves.

Is it a reasonable approach? Is there a less convoluted way to implement the same idea?

Upvotes: 0

Views: 61

Answers (2)

Bitwize
Bitwize

Reputation: 11220

This question is rather subjective, but I'll try to answer in the definitive.

While there isn't anything technically incorrect about the code you have authored, since the code will compile and run correctly, from a higher-level this is a design-smell.

Visibly duplicate calls to std::move on objects limits a developer's ability to reason about the lifetime of that object, which can diminish readability. Static analysis tools may also flag such a move as a "use-after-move", despite the fact that the object has not actually been seated (which technically, would not be wrong).

A couple cleanup options come to mind, but are entirely optional and would depend on your real use-case:

  1. Break out the condition for the std::move into a separate check, or
  2. Use a fallback pattern

Breaking out the condition

If you can break out consumer1's conditions for consuming arg, then you can test this explicitly before you first attempt to consume -- allowing you to have two visibly distinct branches to std::move(arg) independently:

if(can_consume(arg)) {
    consumer1(std::move(arg));
} else {
    consumer2(std::move(arg));
}

Fallback Pattern

An alternative possible refactoring would be to use a fallback pattern, where you design the code such that a dependency-injected "fallback" may occur on failure.

If this is all simply raw functions, a simple example of that could be:

using FallbackFunction = void(*)(Object&&);

void consume1_or(Object&& arg, FallbackFunction function)
{
    if(arg ...) { // arg satisfies some condition
        Object b {std::move(arg)};
        // some processing
        return
    }
    // fallback on failure
    (*function)(std::move(arg));
}

...

// use
consume1_or(std::move(arg), &consume2);

However, this could also extend to a full interface as well, where something that extends Consumer also accepts, on construction, the Consumer to pass objects to on failure.

Upvotes: 2

bipll
bipll

Reputation: 11940

As you have written it, when 1 returns false, then arg is still in a valid state so there's nothing wrong with your code (and even if it wasn't, still there would be nothing wrong).

Upvotes: 1

Related Questions