Error while returning vector member reference

Question

I'm implementing a template Matrix class in c++ and I'm facing an error for operator() overloading. Here is the code:

Matrix.hpp:

template <typename T>
class Matrix {
    std::vector<T>* _data;
    int _sx;
    int _sy;

public:
    // CTOR - DTOR

    Matrix(int sx, int sy);
    Matrix(Matrix<T> const &other);
    ~Matrix();

    // Operator Overloads

    Matrix<T>& operator+=(Matrix<T> const &other);
    Matrix<T>& operator-=(Matrix<T> const &other);
    Matrix<T>& operator*=(Matrix<T> const &other);
    Matrix<T>& operator/=(Matrix<T> const &other);
    Matrix<T>& operator=(Matrix<T> const &other);
    T& operator()(int x, int y);


    // Member Functions
    void display() const;
    int getSx() const;
    int getSy() const;

private: // Private Member functions

    bool _rangeCheck(int x, int y) const;
};

#include "../srcs/Matrix.inl"

Matrix.inl (Problem part)

template <class T>
T& Matrix<T>::operator()(int x, int y) {
    if (this->_rangeCheck(x, y)) {
        return (this->_data[(y * this->_sx) + x]);
    }

    throw std::out_of_range("Bad argument");
}

While compiling I receive this error when doing myMatrix(1, 0) = 1:

./include/../srcs/Matrix.inl:65:9: error: non-const lvalue reference to type 'int' cannot bind to a value of unrelated type 'std::vector<int>'
    return (this->_data[(y * this->_sx) + x]);

Any idea of what I'm doing wrong?

`(*this->_data)[(y * this->_sx) + x]`, I'll take it as a typo. But... why would you need `_data` to be a pointer? — LogicStuff, Oct 11 '16 at 12:52
Template implementation goes in the header. A duplicate of ... somewhere — UKMonkey, Oct 11 '16 at 12:53
`#include "matrix.cpp"` <--- just don't. You'll end up with error just like right now — Guillaume Racicot, Oct 11 '16 at 12:56
@UKMonkey It's [this one](http://stackoverflow.com/questions/495021/why-can-templates-only-be-implemented-in-the-header-file). — πάντα ῥεῖ, Oct 11 '16 at 12:57
Apparently this is not a template in the cpp problem. He was including the cpp. I renamed it to `inl`. — Guillaume Racicot, Oct 11 '16 at 13:00
There is no point in making `_data` a pointer. Only confusion and more potential for errors. — molbdnilo, Oct 11 '16 at 13:06

score 3 · Answer 1 · edited May 23 '17 at 12:33

3

(this->_data[(y * this->_sx) + x]);

Your data is a pointer, so using operator[] on it is accessing data as an array of std::vector<T>. Replace it with:

(*(this->_data)[(y * this->_sx) + x]);

You may consider avoiding pointer here, or inheriting std::vector:

template <typename T>
class Matrix
{
    std::vector<T> _data;
    //...
};

template <typename T>
class Matrix : private std::vector<T>
{
    //...
};

EDIT:

As pointed in the comments by molbdnilo, inheriting a standard container is not recommended.

See this question to understand why.

edited May 23 '17 at 12:33

Community

1
1

answered Oct 11 '16 at 12:57

rocambille

15,398
12
50
68

1

I wouldn't recommend inheritance. The standard collections aren't made to be inherited, not even privately. – molbdnilo Oct 11 '16 at 13:08
@molbdnilo You are right, I was underestimating the consequences here. I edited my answer – rocambille Oct 11 '16 at 13:32

Error while returning vector member reference

1 Answers1