-1

I have just started with C++ and am stuck on the move constructor. Here is my .cpp:

SimpleMatrix::SimpleMatrix(SimpleMatrix &&other_mat) {
 cols = other_mat.cols;
 rows = other_mat.rows;
 data_ = other_mat.data_;
 other_mat.cols = 0;
 other_mat.rows = 0;
 other_mat.data_ = nullptr; // <- Error here
}

I got No viable overloaded = error at other_mat.data_ = nullptr. What went wrong? Is it the way I initialize the matrix?

Here is the relevant parts in .hpp file:

class SimpleMatrix {
 public:
  SimpleMatrix(std::size_t nrows, std::size_t ncols);
  SimpleMatrix(std::initializer_list<std::initializer_list<double>> data);
  SimpleMatrix(SimpleMatrix&& other_mat);
  SimpleMatrix& operator=(SimpleMatrix&& other_mat);

 private:
  std::vector<std::vector<double> > data_;
  int rows, cols;
};
  • 2
    Since all of your types are RAII types there is no need for you to write any of the special member functions. The compiler generated ones will work for you. Also, think about what you are doing in `other_mat.data_ = nullptr;`. Is `data` a pointer? If it isn't, what would it mean to give it the value of `nullptr`? – NathanOliver Dec 28 '18 at 20:03
  • 1
    You also don't need `rows` or `cols`. The vectors known there own sizes, they have a `.size()` member function. – François Andrieux Dec 28 '18 at 20:04
  • `data_ = other_mat.data_;` is performing a copy of the data. You are not doing move construction in your move constructor, you are doing copy construction and then trying to clear the previous object which is not the same thing performance wise. – François Andrieux Dec 28 '18 at 20:05
  • The "relevant parts in `.hpp` file" is missing one very "relevant part": The move-constructor definition itself. – Some programmer dude Dec 28 '18 at 20:11
  • You're right about the missing part @Someprogrammerdude. I have edited it. _Many thanks. – Moritz Graheskl Dec 29 '18 at 07:23

1 Answers1

2

data_ is a vector non-pointer object, and nullptr is to initialize a pointer to be a null pointer.

You can't assign non-pointer variables to be null pointers. And C++ doesn't have any concept of null values or objects.

If you want the vector to be properly initialized I suggest you add a constructor initializer list:

SimpleMatrix::SimpleMatrix(SimpleMatrix &&other_mat)
    : data_(std::move(other_mat.data_))  // Move the data from the other vector
    , rows(other_mat.rows)
    , cols(other_mat.cols)
{
    // Clear the other matrix rows and cols
    other_mat.rows = 0;
    other_mat.cols = 0;
}

Or, you could rely on the rule of zero and let the compiler-generated constructors handle everything for you, which in this case it should do properly:

class SimpleMatrix {
 public:
  SimpleMatrix(SimpleMatrix &&) = default;
  // ...
};
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • 1
    Well, not entirely properly: the moved-from object will have an empty vector along with non-zero `rows` and `cols` variables. To zero those without losing the compiler-assisted move constructor generation, a private base class could be used: https://rextester.com/SURN53114 – Ben Voigt Dec 28 '18 at 20:28
  • This solves my problem. But more than that, thanks for a clear and super informative answer – Moritz Graheskl Dec 29 '18 at 07:42