Reputation: 3
I am trying to write Matrix class. I am using unique_ptr as a storage pointer array. But this unique_ptr is private member of class. I want to add move constructor to my Matrix class. But I need getter function for private unique_ptr but when I return unique_ptr as a const variable I can not use std::move but when I return getter function without const reference compiler doesn't not allow this.
Matrix.hpp
template <typename _T>
class Matrix_
{
private:
size_t rows_;
size_t cols_;
size_t size_;
std::unique_ptr<_T[]> data_;
public:
Matrix_();
Matrix_(const Matrix_& _m); //Copy constructor
Matrix_(Matrix_&& _m) noexcept; //Move constructor
Matrix_(const size_t& rows);
Matrix_(const size_t& rows,const size_t& cols);
Matrix_(const size_t& rows,const size_t& cols,const _T& val);
//Operators
Matrix_& operator=(const Matrix_& _m); //Copy assignment
Matrix_& operator=(Matrix_&& _m) noexcept; //Move assignment
//
//Methods
size_t getRows()const;
size_t getCols()const;
size_t getSize()const;
const std::unique_ptr<_T[]>& getData()const;
void copyData(const std::unique_ptr<_T[]>& _data);
void fill(const _T& val);
//
~Matrix_();
};
template <typename _T>
Matrix_<_T>::Matrix_():rows_(1),cols_(1),size_(rows_*cols_),data_(std::make_unique<_T[]>(size_))
{
}
template <typename _T>
Matrix_<_T>::Matrix_(const Matrix_& _m):rows_(_m.getRows()),cols_(_m.getCols()),size_(rows_*cols_),data_(std::make_unique<_T[]>(size_))
{
}
template <typename _T>
Matrix_<_T>::Matrix_(Matrix_&& _m)noexcept:rows_(_m.getRows()),cols_(_m.getCols()),size_(rows_*cols_),data_(std::move(_m.getData()))
{
}
template <typename _T>
Matrix_<_T>::Matrix_(const size_t& rows):rows_(rows),cols_(1),size_(rows_*cols_),data_(std::make_unique<_T[]>(size_))
{
}
template <typename _T>
Matrix_<_T>::Matrix_(const size_t& rows,const size_t& cols):rows_(rows),cols_(cols),size_(rows_*cols_),data_(std::make_unique<_T[]>(size_))
{
}
template <typename _T>
Matrix_<_T>::Matrix_(const size_t& rows,const size_t& cols,const _T& val):rows_(rows),cols_(cols),size_(rows_*cols_),data_(std::make_unique<_T[]>(size_))
{
fill(val);
}
//Operators
template <typename _T>
Matrix_<_T>& Matrix_<_T>::operator=(const Matrix_& _m)
{
rows_ = _m.rows_;
cols_ = _m.cols_;
size_ = rows_*cols_;
data_ = std::make_unique<_T[]>(size_);
copyData(_m.getData());
return *this;
}
template <typename _T>
Matrix_<_T>& Matrix_<_T>::operator=(Matrix_&& _m)noexcept{
rows_ = _m.rows_;
cols_ = _m.cols_;
size_ = rows_*cols_;
data_ = std::move(_m.getData());
return *this;
}
//
//Methods
template <typename _T>size_t Matrix_<_T>::getRows()const{return rows_;}
template <typename _T>size_t Matrix_<_T>::getCols()const{return cols_;}
template <typename _T>size_t Matrix_<_T>::getSize()const{return size_;}
template <typename _T>const std::unique_ptr<_T[]>& Matrix_<_T>::getData()const{return data_;}
template <typename _T>void Matrix_<_T>::copyData(const std::unique_ptr<_T[]>& _data){
for(uint i=0;i<size_;i++){data_[i]=_data[i];}
}
template <typename _T>void Matrix_<_T>::fill(const _T& val){
for(uint i=0;i<size_;i++){data_[i]=val;}
}
//
template <typename _T>
Matrix_<_T>::~Matrix_()
{
}
Test.cpp
int main()
{
Matrix_<double> a(10,10,5.0);
Matrix_<double> b(10,10);
b = std::move(a);
std::cout<<b.getData()[0]<<std::endl;
return 0;
}
Error
error: use of deleted function ‘std::unique_ptr<_Tp [], _Dp>& std::unique_ptr<_Tp [], _Dp>::operator=(const std::unique_ptr<_Tp [], _Dp>&) [with _Tp = double; _Dp = std::default_delete<double []>]’
data_ = std::move(_m.getData());
~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/7/bits/locale_conv.h:41:0,
from /usr/include/c++/7/locale:43,
from /usr/include/c++/7/iomanip:43,
from /home/cemo/YZlib/Examples/Test.cpp:3:
/usr/include/c++/7/bits/unique_ptr.h:654:19: note: declared here
unique_ptr& operator=(const unique_ptr&) = delete;
Upvotes: 0
Views: 397
Reputation: 72271
Don't use the getter in your move constructor or move assignment. Just use the member directly. I would probably do this for all members, to make it obvious they match up.
template <typename T>
Matrix<T>::Matrix(Matrix &&m) noexcept :
rows_(m.rows_), cols_(m.cols_), size_(m.size_), data_(std::move(m.data_))
{
// changes needed to m?
}
template <typename T>
Matrix<T>& Matrix<T>::operator=(Matrix &&m) noexcept
{
rows_ = m.rows_;
cols_ = m.cols_;
size_ = m.size_;
data_ = std::move(m.data_);
// changes needed to m?
return *this;
}
A class always has access to its own members (no matter which object they're accessed through).
Be sure to consider what should happen to m
now that its data_
is a null pointer. Should its rows_
, cols_
, and size_
be set to zero, or doesn't it matter? Normally a moved-from object should be left in a "valid but unspecified" state, whatever you decide that means for your class. At a minimum, calling the destructor should be valid, and assigning it to something else should work as expected; these are already the case.
Since the getter isn't needed here, it's a separate question whether the public interface should include a getter at all, or if it should have a more convenient return type like just const T*
or T*
.
Upvotes: 1
Reputation: 9331
Doing:
template <typename _T>
Matrix_<_T>& Matrix_<_T>::operator=(Matrix_&& _m) noexcept {
rows_ = _m.rows_;
cols_ = _m.cols_;
size_ = rows_*cols_;
data_ = std::move(_m.data_);
return *this;
}
Will fix the code. You don't need to use getter function inside the class, only when you want to access the member outside the class.
Upvotes: 0