Как правильно оформить присваивание значения возвращаемого из функции C++?

Question

[Urilobus]

Создал класс матриц в С++, есть конструктор копирования:

Matrix (Matrix &Mat)
{
    this->cols_num = Mat.cols_num;
    this->rows_num = Mat.rows_num;
    Mat_1 = new double*[Mat.cols_num];
    for (int i = 0; i < Mat.cols_num; i++)
    {
        Mat_1[i] = new double[Mat.rows_num];
        for (int j = 0; j < Mat.rows_num; j++) Mat_1[i][j] = Mat(i,j);
    } 
    std::cout<<"Копирование "<<this<<std::endl;
}

Оператор присваивания:

Matrix& operator = (Matrix& Mat)
    {
        double **Mat_copy;
        this->cols_num = Mat.cols_num;
        this->rows_num = Mat.rows_num;
        Mat_copy = new double*[Mat.cols_num];
        for (int i = 0; i < Mat.cols_num; i++)
        {
            Mat_copy[i] = new double[Mat.rows_num];
            for (int j = 0; j < Mat.rows_num; j++) Mat_copy[i][j] = Mat(i,j);
        }
        Mat_1 = Mat_copy;
        return *this;
    }

И функция транспонирования

Matrix Transpose(Matrix &Mat)
{
    Matrix Mat_trans(Mat.cols_num, Mat.rows_num); 
    for(int i = 0; i < Mat_trans.cols_num; i++)
        for (int j = 0; j < Mat_trans.rows_num; j++)
            Mat_trans(i,j) = Mat(j,i); 
    return Mat_trans;    
}

Простое присваивание типа Mat1 = Mat2, работает корректно, но при попытке присвоить значение возвращаемое функцией транспонирования Mat1 = Transpose(Mat2) возникает ошибка (no operator "=" matches these operands)
Как правильно вытащить транспонированную матрицу из функции Transpose?

Comments

[mayton2019]

Конструктор и оператор присвоения отличаются двумя строчками. Рефакторинг просится. Или у вас, С++ ников не делают рефакторинг?

[res2001]

И в конструкторе копирования и в операторе присваивания вы не проверяете выделена ли уже память в Mat_1, просто перезаписываете указатель. Из-за этого, если объект уже был ранее сконструирован и память выделена, то будет утечка памяти.

Ну и если вам не безразлично быстродействие вашей программы, то выделяйте память одним большим куском размером Mat.rows_num * Mat.cols_num, а не как сейчас - под каждую строку матрицы свой массив. Дело в том, что в вашем случае каждая строка будет располагаться в памяти где-то в своем месте, не обязательно рядом с другими строками. Когда выделите память одним куском, то весь массив будет находится в одном месте и попаданий в кэш будет гораздо больше, что положительно скажется на производительности операций с матрицей.

[Евгений Шатунов]

mayton2019

Или у вас, С++ ников не делают рефакторинг?

У кого?

[Urilobus]

res2001, Спасибо за ответ. Насчёт выделение памяти сразу большим блоком, я поступил иначе, т.к. предполагается работа с большими массивами данных и возможно, что в памяти не будет иметься столько свободного места одним блоком, если я правильно понимаю механику работы

[res2001]

Urilobus, Скорее всего вы не правильно понимаете, т.к. память виртуальная и в 64битных системах большой запас виртуального адресного пространства, а если ваши матрицы не будут влазить в физическую память - не важно, память то виртуальная.
Если же у вас будут большие разреженные матрицы, то вам нужно использовать другие структуры для хранения данных, скорее всего хэш таблицы.

Answer 1

[Wataru]

Потому что аргументы должны быть const Matrix&.

Возвращаемое функцией значение является временным значением. В него нельзя писать, но его можно читать. Оно const. И его нельзя передавать в описанный вами оператор копирования.

Comments

[Евгений Шатунов]

Wataru

В него нельзя писать, но его можно читать. Оно const.

Вот тут неправда.

Понимание этого тонкого момента зависит от понимания категорий выражений в языке.
Для метода Matrix Transpose(Matrix &Mat) будет возвращено локальное временное значение с категорией prvalue.
Значения с такой категорией лишены идентичности, динамического типа и размещения. Их нельзя передать по неконстантной ссылке. Читать и писать в него можно при этом.
Именно поэтому у автора транслятор не может найти подходящий оператор. У него и копирование сделано через оператор разрушающего копирования, и метод транспонирования может модифицировать состояние аргумента. А это - в корне неправильно.

[Wataru]

Евгений Шатунов,

Читать и писать в него можно при этом.

А как в них можно писать, если они "лишены идентичности, динамического типа и размещения"? Можете привести пример?

[Евгений Шатунов]

Wataru, а вот так:
Matrix out{ Transpose( in ).Inverse() };
Это при том, что метод Inverse может иметь сигнатуру Matrix& Matrix::Inverse(); и возвращать *this для того чтобы позволить Method chaining для объекта матрицы.

И ровно в это же время для кода Matrix out{ Transpose( in ) }; будет вызван не конструктор копирования, а именно конструктор перемещения, аргументом которого является Matrix&&. Такое приведение невозможно для константных объектов. А prvalue можно использовать для инициализации rvalue reference.

[Urilobus]

Евгений Шатунов, Вы правы, дело в категории prvalue. Благодарю за статью. Я отказался от способа передачи по константной ссылке, так как в таком случае у меня в конструкторе копирования и в операторе присваивания при передачи в них аргумента (const Matrix& Mat) выдавал ошибку при попытке обратиться к элементу матрицы Mat(i,j);
оператор для обращения к элементу прописан так

double& operator()(int index1, int index2)
    {
	    return Mat_1[index1][index2];
    }

[Wataru]

Urilobus, вам надо сделать 2 версии оператора () в Matrix. Одна должна быть помечена const и возвращать константную ссылку. Тогда все заработает.

[Urilobus]

Wataru, Если я правильно понимаю, то нельзя перегрузить оператор, отличающийся только типом возвращаемого значения

[Wataru]

Urilobus, да, но надо кроме типа значения пометить сам оператор const. После параметров, вот так:
const &m operator() const;

[Urilobus]

Wataru, Огромное спасибо!!!