Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

C++ template copy constructor, compiler says "passing const as this argument discards qualifiers"

I'm trying to create a template class of dynamic matrices. With my current knowledge of C++ I managed to solve some problems but I'm stuck at copy constructor and overloading operator=; in other words, I cannot create copies of my objects. In my oppinion this should work, but my friend, compiler, tells me that I have 1 error: error: passing ‘const Matrix’ as ‘this’ argument of ‘int Matrix::getElement(int, int) [with T = int]’ discards qualifiers [-fpermissive] at this line:

m[i][j] = original.getElement(i, j);

when I want to createe an object:

Matrix<int> m = Matrix<int>(3, 3);

My template class is here:

template<class T>class Matrix
{
 public:

  Matrix<T>(int lines, int columns)
  {
    this->lines = lines;
    this->columns = columns;
    T* aux = new T[this->lines * this->columns];
    m = new T*[lines];
    for (int i = 0; i < this->lines; i++)
    {
      m[i] = aux + (i * this->columns);
    }
    for (int i = 0; i < this->lines; i++)
    {
      for (int j = 0; j < this->columns; j++)
      {
        m[i][j] = 0;
      }
    }
  }

  Matrix<T>(const Matrix<T>& original)
  {
    columns = original.getColumns();
    lines = original.getLines();
    T* aux = new T[this->lines * this->columns];
    m = new T*[lines];
    for (int i = 0; i < lines; i++)
    {
      m[i] = aux + (i * this->columns);
    }
    for (int i = 0; i < lines; i++)
    {
      for (int j = 0; j < columns; j++)
      {
        m[i][j] = original.getElement(i, j);
      }
    }
  }

  virtual ~Matrix<T>()
  {
    /*for (int i = lines - 1; i > 0; i--)
    {
      delete m[i];
    }*/
    delete [] m;
  }

  T** getPointer()
  {
    return m;
  }

  int getLines () const
  {
    return lines;
  }

  int getColumns () const
  {
    return columns;
  }

  int getElement(int line, int column)
  {
    return m[line][column];
  }

  int setElement(int line, int column, T value)
  {
    m[line][column] = value;
  }

  Matrix<T>* getTranspose()
  {
    Matrix<T>* aux = new Matrix<T>(lines, columns);
    for (int i = 0; i < lines; i++)
    {
      for (int j = 0; j < columns; j++)
      {
        aux->setElement(i,j, m[j][i]);
      }
    }
    return aux;
  }

  Matrix<T> operator=(const Matrix<T> original)
  {
    columns = original.getColumns();
    lines = original.getLines();
    T* aux = new T[this->lines * this->columns];
    m = new T*[lines];
    for (int i = 0; i < lines; i++)
    {
      m[i] = aux + (i * this->columns);
    }
    for (int i = 0; i < lines; i++)
    {
      for (int j = 0; j < columns; j++)
      {
        m[i][j] = original.getElement(i, j);
      }
    }
  }

  friend std::ostream& operator<<(std::ostream& out, Matrix<T>& matrix)
  {
    out<<"Matrix:"<<std::endl;
    for (int i = 0; i < matrix.getLines(); i++)
    {
      for (int j = 0; j < matrix.getColumns(); j++)
      {
        out<<matrix.getElement(i, j)<<" ";
      }
      out<<std::endl;
    }
    return out;
  }

  friend std::istream& operator>>(std::istream& in, Matrix<T>& matrix)
  {
    std::cout << "Reading Matrix:\n";
    for (int i = 0; i < matrix.getLines(); i++)
    {
      for (int j = 0; j < matrix.getColumns(); j++)
      {
        std::cout << "Matrix[" << i << "][" << j << "]:";
        in >> matrix.m[i][j];
      }
      std::cout << std::endl;
    }
    return in;
  }

 private:
  T** m;
  int lines;
  int columns;
};

As I can figure out from that error I'm creating 2 objects which refers to same block of memory, but I want to create 2 objects which refers to 2 different blocks of memory with same contents.

like image 291
David Avatar asked Dec 21 '22 14:12

David


2 Answers

In your copy constructor

Matrix<T>(const Matrix<T>& original)

original is declared as const reference, and that is very good. However, it means that you must not call any methods of Matrix<T> on it that are not declared as const functions.

The getElement function is not declared as const so you can't use it inside the copy constructor. Solve this by declaring it as a const-function:

int getElement(int line, int column) const  // <--- Note the 'const'
{
  return m[line][column];
}

What this means is:

  1. This function can be called on Matrix objects that are const (such as original in your copy constructor)

  2. You cannot perform any action inside getElement that modifies the current instance of Matrix (i.e. that modifies *this).

The former is what we want, and the latter is not a problem because getElement is just a getter method, so it needs not modify anything.

Note that because of this, it is a good idea to always mark member functions as const when they are not supposed to modify anything. It means you can apply them to constant objects, and it means that the compiler will tell you if, by mistake, you put code into the function that actually does modify something.

Final remark: As pointed out by Tore Olsen, the getElement function should probably return an object or reference of type T rather than int.

like image 97
jogojapan Avatar answered Dec 23 '22 02:12

jogojapan


As I use to say, when you start to write const, you cannot stop until the end. Because a const object can use const method and only const method. However, the const-correctness is important for a C++ programmer. I read an article about that once, unfortunatly it was in French but I think it is easy to found in English. It explained that const-correctness brings safety and security in your programm. And it helps for some optimizations, but here I cannot remember exactly the author's point.

like image 37
lthms Avatar answered Dec 23 '22 04:12

lthms