0

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;
  • *Why* do you need a getter for internal data representation and *who* should be the owner of this data after getter call? Should it still be owned by `Matrix_` or should getter caller become the sole owner of data? – Yksisarvinen Mar 29 '20 at 14:50
  • owner must be same I need getter for reaching data member of arguments for example 'Matrix_<_t>::Matrix_(Matrix_&& _m)' I need reach data_ variable of _m – YlmzCmlttn Mar 29 '20 at 14:52
  • That's a *member* function. Members functions have access to all private members of the class (no matter if you want to access the same or different object, class is the same). You'd only need a getter if you need to access it from *outside* of the class (a.k.a. any function which is not a member of `Matrix_`) – Yksisarvinen Mar 29 '20 at 14:59
  • Thank you. I didn't know that. But still I wonder why I can not move with using getData() I return private member as a reference – YlmzCmlttn Mar 29 '20 at 15:02
  • Don't use an identifier starting with an underscore followed by capital letter; these are reserved for the implementation. Don't forget to make the move assignment `noexcept` also. – aschepler Mar 29 '20 at 15:03

2 Answers2

1

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*.

aschepler
  • 65,919
  • 8
  • 93
  • 144
  • "A class always has access to its own members (no matter which object they're accessed through)." I didn't know that thanks. But why std::move(m.getData()) don't work. I don't understand. So if I use getter function outside of class. For example. 'std::unique_ptr c = std::make_unique(100); c = std::move(a.getData());' How I can move data to another smart pointer – YlmzCmlttn Mar 29 '20 at 15:19
  • Because `m.getData()` is `const`, and you can't normally move from a `const` expression. You would need a getter which is not `const` and which had return type `std::unique_ptr&` or `std::unique_ptr&&`. Which is then not so much a getter, but a method for giving away data. – aschepler Mar 29 '20 at 15:22
0

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.

Waqar
  • 6,944
  • 2
  • 26
  • 39