1
\$\begingroup\$
template<typename T>
class Vector
{
private:
    size_t size;
    size_t capacity;
    T* buffer;
public:
    Vector(size_t s) {
        size = 0;
        capacity = s;       
        buffer = new T[capacity];
    }
    //Move constructor
    Vector(Vector&& rhs): size(rhs.size),
                          capacity(rhs.capacity),
                          buffer(tmp.buffer)
    {
        rhs.buffer = nullptr;
    }
    T& operator=(Vector&& tmp) {
        size = tmp.size;
        capacity = tmp.capacity;
        buffer = std::move(tmp.buffer);
        return *this;
    }
    void push_back(const T& v) {
        if (size >= capacity)
            reserve(capacity + 5);
        buffer[size++] = v;
    }
    void push_back(T&& v) {
        if (size >= capacity)
            reserve(capacity + 5);
        buffer[size++] = v;
    }

Have I implemented all move operations in my class Vector? Is it correct implementation of method push_back for rvalue ?

\$\endgroup\$

closed as off-topic by 200_success Mar 27 '17 at 19:05

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 1
    \$\begingroup\$ Welcome to Code Review! We don't check for correctness of the code. Please refer to help center if you have further questions. Also, your vector default constructs everything. You need to use uninitialized memory, aka Heil malloc. \$\endgroup\$ – Incomputable Mar 26 '17 at 21:51
  • \$\begingroup\$ Have you tested the code? It's up to you to tell us if the code works correctly. \$\endgroup\$ – Phrancis Mar 27 '17 at 0:11
  • \$\begingroup\$ Put on hold because buffer(tmp.buffer) in the move constructor won't compile. \$\endgroup\$ – 200_success Mar 27 '17 at 19:06
  • \$\begingroup\$ Your assignment leaks. Have a read of this: lokiastari.com/blog/2016/02/27/vector I prefer using the swap technique for move semantics. It delays destruction of resources thus potentially allowing re-use. \$\endgroup\$ – Martin York Mar 28 '17 at 21:29
1
\$\begingroup\$

Have I implemented all move operations in my class Vector?

Yes. And no. You have a bug. You are not resetting the size of the moved Vector, but resetting its buffer, which can break code like this:

Vector<int> vec;
vec.push_back(1);
Vector<int> v = std::move(vec);
if (vec.size() == 1)
    vec[0] = 8; // undefined behavior!

Also, what is tmp? Did you mean to use rhs?

Is it correct implementation of method push_back for rvalue ?

No. In

buffer[size++] = v;

you are not taking advantage that v is a rvalue reference. You are copying v into the buffer, but that is not needed, as v points to a rvalue. You can move it to the buffer:

buffer[size++] = std::move(v);

Although you have implemented all move operations for Vector, you forgot the copy operations. But maybe you just hide them because you didn't want any feedback on them.

Nonetheless, you could have written your move constructor like this, which is more readable IMO (and doesn't include the bug mentioned above):

Vector(Vector&& rhs) : size(std::exchange(rhs.size, 0)),
                       capacity(std::exchange(rhs.capacity, 0)),
                       buffer(std::echange(rhs.buffer, nullptr))
{}

You can do the same thing for operator=(Vector&&).

However, consider using the copy-swap idiom, for a exception safe way of implementing the Rule of Five.

\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.