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

Is this a correct implementation/use of move semantics?

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

Problem

I'm trying to account for the fact that my program will contain calculations such as:

v(t) = w ∗ v(t−1)+ c1∗r1∗( pBest − x(t −1)) + c2∗r2∗(gBest − x(t−1))


where v, x, pBest, gBest are vectors (std::arrays). I'm a little worried if I make correct use of the move semantics.

Here's the relevant snippet:

template std::array operator *(std::array&& lhs, const F rhs ) {

    static_assert(std::is_arithmetic::value, "The type parameter of the first argument is not of arithmetic type");
    static_assert(std::is_arithmetic::value, "The passed scalar is not of arithmetic type");

    std::array tmp = std::move(lhs);

    for(size_t i = 0; i  std::array operator *(const F rhs, std::array&& lhs ) {

    return std::move(lhs) * rhs;
}


POST-ANSWER EDIT: As Loki Astari pointed out this SHOULD NOT be the sole implementation for operator* as this can bind to lvalues and will destroy them.

Solution

Yes BUT.

The std::array itself is not moveable. But it is still optized for move semantics when its content type T is moveable. So it will move each of its internal elements rather than copy them. But your function limits the type of T to std::is_arithmetic which is only defined for built-in integers and floats. Moving integers and floats is the same as copying them. So you get no real benefit.

But lets take this a step further.

If you remove the static_assert so it can be used by any type that supports the operator* etc.. then you have a usability issue.

When you do a multiplication like this you destroy the original array (Its content has been moved so there is no way to know what the elements are or if they are usable. About the only thing you can do is reset them to a specific state).

#include 

int main()
{
    std::array  a = initArray();
    std::array  c;

    // Your code moves the content of a here.
    c = a * 5;

    // So at this point the content of a is in an undefined state
    // So you can not rely on what is in `a` or use it in ANY way that would
    // need the content to be in a defined state (all you can really do is reset).
    //
    // This is not what I would expect of `operator*`
    // So it is quite easy to make mistakes with this code.
}

Code Snippets

#include <array>

int main()
{
    std::array<MyMathType, 5>  a = initArray();
    std::array<MyMathType, 5>  c;

    // Your code moves the content of a here.
    c = a * 5;

    // So at this point the content of a is in an undefined state
    // So you can not rely on what is in `a` or use it in ANY way that would
    // need the content to be in a defined state (all you can really do is reset).
    //
    // This is not what I would expect of `operator*`
    // So it is quite easy to make mistakes with this code.
}

Context

StackExchange Code Review Q#66927, answer score: 6

Revisions (0)

No revisions yet.