перегрузка оператора * для умножения матриц

Хочу перегрузить оператор * для класса матриц, который позволит писать с=a*b;где c,a,b матрицы (в классе они реализованы через двумерные динамические массивы). Может быть, я где-то накосячил с указателями, но почему-то при перемножении выводятся какие-то левые (мусорные) значения. Вот код:

#include <iostream>
#include <math.h>

using namespace std;

class matrix
{
private:
    int rows;
    int columns;
    double **elements;
public:
    matrix(int rows, int columns)
    {
        this->rows = rows;
        this->columns = columns;
        elements = new double*[rows];
        for (int i = 0; i < rows; i++)
        {
            elements[i] = new double[columns];
        }
    }
void set_elements(); //тут всё работает, поэтому расписывать не буду, чтобы не запутать читающего
void get_matrix();   //тут всё работает, поэтому расписывать не буду, чтобы не запутать читающего

matrix& operator = (const matrix& other) //перегрузка оператора присвоения
    {                                          //мб тут ошибка
        this->rows = other.rows;
        this->columns = other.columns;
        if (this->elements != nullptr)
        {
            for (int i = 0; i < rows; i++)
            {
                delete[]elements[i];
            }
            delete[]elements;
        }
        this->elements = new double*[other.rows];
        for (int i = 0; i < other.rows; i++)
        {
            for (int j = 0; j < other.columns; j++)
            {
                this->elements[i][j] = other.elements[i][j];
            }
        }
        return *this;
    }
matrix& operator * (const matrix& other) //вот тут программа болеет. other = вторая (правая) умножаемая матрица
    {
        if (this->columns == other.rows)  //проверка, можно ли перемножать матрицы
        {
            double** tmpmatrix = new double* [this->rows]; //создаём промежуточный массив указателей (двумерный) длиной в кол-во строк первой (левой) умножаемой матрицы
            for (int i = 0; i < this->rows; i++)           //создаём собственно строки промежуточной матрицы длиной а количество строк второй (правой) умножаемой матрицы
            {
                tmpmatrix[i] = new double[other.columns]; 
            }
            for (int i = 0; i < this->rows; i++)  //тут 3 цикла, чтобы сложить поэлементно строки и столбцы левой и правой матриц и записать их в промежуточную матрицы
            {
                for (int j = 0; j < other.columns; j++)
                {
                    for (int k = 0; k < other.rows; k++)
                    {
                        tmpmatrix[i][j] += this->elements[i][k] * other.elements[k][j]; //почему-то подчёркивается зелёным цветом, якобы тут ошибка
                    }
                }
            }
            for (int i = 0; i < this->rows; i++) //удаляем первую (левую) умножаемую матрицу
            {
                delete[]this->elements[i]; 
            }
            delete[]this->elements; 
            this->columns = other.columns;  //свойству columns возвращаемой матрицы передаём значение кол-ва столбцов правой матрицы 
            this->elements = tmpmatrix;  //свойству elements передаём указатель на новую матрицу
            return *this; 
        }
        else
        {
            cout << "Ошибка. Невозможно перемножить эти матрицы (кол-во столбцов первой матрицы не равно кол-ву строк второй)\a\n";
        }
    }
~matrix()
    {
        for (int i = 0; i < rows; i++)
        {
            delete[]elements[i];
        }
        delete[]elements;
    }
};

int main()
{
    setlocale(LC_ALL, "Rus");
    matrix a(2, 3);
    a.set_elements(); //вводим элементы матрицы
    matrix b(3, 2);
    b.set_elements(); //вводим элементы матрицы
    matrix c = a * b;
    c.get_matrix(); //выводим получившуюся матрицу
    return 0;
}

Ответы (2 шт):

Автор решения: AVK

Ваш оператор умножения записывает произведение матриц на место левой матрицы вместо того, чтобы его вернуть. Когда выполняется строка this->elements = tmpmatrix, вместо матрицы левого множителя записывается матрица произведения. После этого возвращается ссылка на объект левого множителя. Так делать нельзя не только потому, что матрица a в вашем коде портится, но и потому, что так можно получить висячую ссылку когда объект левого множителя будет уничтожен. Оператор умножения должен не возвращать ссылку, а создавать новый объект и возвращать его не по ссылке. Как-то так:

matrix operator * (const matrix& other) const
{
    matrix ret(rows,other.columns);
    if (this->columns == other.rows)  //проверка, можно ли перемножать матрицы
    {
        for (int i = 0; i < rows; i++)  
        {
            for (int j = 0; j < other.columns; j++)
            {
                double tmp= 0;
                for (int k = 0; k < other.rows; k++)
                {
                    tmp += get_element(i,k) * other.get_element(k,j);
                }
                ret.set_element(i,j,tmp);
            }
        }
    }
    else
    {
        cout << "Ошибка. Невозможно перемножить эти матрицы (кол-во столбцов первой матрицы не равно кол-ву строк второй)\a\n";
    }
    return ret;
}

get_element и set_element - функции чтения и записи элементов матрицы.

Другой нехороший момент в вашем классе это то, что в нём не определены копирующий конструктор, конструктор перемещения и оператор присваивания. В результате код типа matrix c = a * b; будет работать неправильно, копируя указатель, а не данные. Класс надо модифицировать, например, так:

matrix(const matrix&)= delete;
matrix& operator=(const matrix&)= delete;
matrix(matrix&& other)
{
    rows= other.rows;
    columns= other.columns;
    elements= other.elements;
    other.rows= 0; // чтобы деструктор правильно сработал
    other.elements= nullptr;
}
→ Ссылка
Автор решения: Ezhik

Всем спасибо за советы. Вот что в итоге получилось. Сделать матрицу в виде одного массива размером количество_строк*количество_столбцов действительно удобнее и эффективнее. Конкретно по теме умножения матриц код получился такой:

#include <iostream>
#include <math.h>
using namespace std;

template <class T>
class matrix
{
private:
    int rows;
    int columns;
    int all_elements_num;
    T* elements;
public:

    matrix();  //конструктор по умолчанию
    matrix(int rows, int columns);  //конструктор создания матрицы
    void cin_elements();  //ввод элементов с клавиатуры
    void cout_elements();  //вывод элементов в консоль
    T get_element(int row_number, int column_number) const;  //метод, возвращающий элемент указанной строки и столбца
    matrix<T>& operator = (const matrix<T>& other_matrix); //перегрузка оператора присваивания (левой матрице присваиваются значения элементов правой)
    matrix<T> operator * (const matrix<T>& other_matrix); //перегрузка оператора * матрицы на другую матрицу
    ~matrix();   //деструктор
};

template <class T>
matrix<T>::matrix()   //конструктор по умолчанию
{
    rows = 1;
    columns = 1;
    all_elements_num = 1;
    elements = new T[1];
    elements[0] = 0;
    cout << "Вызвался конструктор по умолчанию для объекта " << this << endl;
}

template <class T>
matrix<T>::matrix(int rows, int columns)   //конструктор создания матрицы
{
    this->rows = rows;
    this->columns = columns;
    all_elements_num = rows * columns;
    elements = new T[all_elements_num];
    for (int i = 0; i < all_elements_num; i++)
    {
        elements[i] = 0;
    }
    cout << "Вызвался конструктор для объекта " << this << endl;
}

template <class T>
void matrix<T>::cin_elements()  //ввод элементов с клавиатуры
{
    cout << "Введите построчно элементы матрицы\n";
    for (int i = 0; i < all_elements_num; i++)
    {
        cin >> elements[i];
    }
}

template <class T>
void matrix<T>::cout_elements()  //вывод элементов в консоль
{
    int counter = 0;
    for (int i = 0; i < rows; i++)
    {
        for (int j = 0; j < columns; j++)
        {
            cout << elements[counter] << " ";
            counter++;
        }
        cout << endl;
    }
}

template <class T>
T matrix<T>::get_element(int row_number, int column_number) const  //метод, возвращающий элемент указанной строки и столбца
{
    if (((columns * (row_number - 1) + (column_number - 1)) >= 0) && ((columns * (row_number - 1) + (column_number - 1)) < all_elements_num))
    {
        return elements[columns * (row_number - 1) + (column_number - 1)];
    }
    else
    {
        cout << "Ошибка в обращении к элементу матрицы \a\n";
        return 0;
    }
}

template <class T>
matrix<T>& matrix<T>::operator = (const matrix<T>& other_matrix)   //перегрузка оператора присваивания (левой матрице присваиваются значения элементов правой)
{
    if ((this->rows == other_matrix.rows) && (this->columns == other_matrix.columns))
    {
        for (int i = 0; i < this->all_elements_num; i++)
        {
            this->elements[i] = other_matrix.elements[i];
        }
        return *this;
    }
    else
    {
        cout << "Ошибка, размеры матриц не соответствуют\a\n";
        return *this;
    }
}

template <class T>
matrix<T> matrix<T>::operator * (const matrix<T>& other_matrix)   //перегрузка оператора * матрицы на другую матрицу
{
    if (this->columns == other_matrix.rows)
    {
        int counter = 0;
        matrix<T> tmp(this->rows, other_matrix.columns);
        for (int i = 1; i <= this->rows; i++)
        {
            for (int j = 1; j <= other_matrix.columns; j++)
            {
                for (int k = 1; k <= this->columns; k++)
                {
                    tmp.elements[counter] += this->get_element(i, k) * other_matrix.get_element(k, j);
                }
                counter++;
            }
        }
        return tmp;
    }
    else
    {
        cout << "Ошибка, размеры матриц не соответствуют, невозможно выполнить умножение\a\n";
        matrix<T> tmp;
        return tmp;
    }
}

template <class T>
matrix<T>::~matrix()  //деструктор
{
    if (elements != nullptr)
    {
        delete[]elements;
    }
    cout << "Вызвался деструктор для объекта " << this << endl;
}

int main()
{

return 0;
}

→ Ссылка