При добавлении двух матриц печатается столбец данных мусора c ++

Question

Я пытаюсь добавить две матрицы, используя многомерные массивы и перегружая оператор сложения '+' и оператор присваивания '=', однако данные мусора передаются в мою перегруженную функцию оператора присваивания.

Я читал кое-что о идиомах копирования и обмена и т. Д., Но не могу найти решение своей проблемы, и я предпочел бы использовать + и = отдельно, а не '+ =' что я видел, как делают некоторые люди, потому что я буду использовать другую арифметику для матриц, но сначала я хочу решить эту проблему.

Вот мой заголовок:

#ifndef Matrix_h
#define Matrix_h
#include <stdio.h>
#include <stdlib.h>
#include <iostream>
#include <utility>

class Matrix
{
private:
    int row;
    int column;
    double  ** elements;

public:
    Matrix();
    Matrix(int r, int c); //constructor
    Matrix(const Matrix& src);  //copy constructor
    ~Matrix(); //destructor

    Matrix& operator=(const Matrix& right);  // assignment operator
    int getRow() const;
    int getColumn() const;
    void setElement(int r, int c, double data);
    double getElement(int r, int c) const;
    Matrix& operator+(const Matrix& right); // calculates the sum of two matrices

};

#endif 

Вот мои определения функций:

#include <string.h>
#include "matrix.h"
using namespace std;

Matrix::Matrix(int r, int c){ //defines the constructor to create a new matrix
    row = r;
    column = c;
    elements = new double*[row];
    for (int i=0; i<row; i++){
        elements[i] = new double[column];
    }
    for (int i=0; i<row; i++){
        for (int j=0; j<column; j++){
            elements[i][j] = 0;
        }
    }
}

Matrix::Matrix(const Matrix& src){ //defines the copying constructor
    row = src.row;
    column = src.column;
    elements = new double*[row];
    for (int i=0; i<row; i++){
        elements[i] = new double[column];
    }
    for (int i=0; i<row; i++){
        for (int j=0; j<column; j++){
            elements[i][j] = src.elements[i][j];
        }
    }
}

Matrix::~Matrix() { //defines the destructor
    for (int i=0; i<row; i++){
        delete[] elements[i];
    }

    delete[] elements;
};

void Matrix::setElement(int r, int c, double data) {
    elements[r][c] = data;
};

double Matrix::getElement(int r, int c) const {
    return elements[r][c];
}

int Matrix::getColumn() const {
    return column;
}

int Matrix::getRow() const {
    return row;
}

Matrix& Matrix::operator =(const Matrix& right) {

    if(this->elements != right.elements && column==right.column && row==right.row)
    {
        memcpy ( &this->elements, &right.elements, sizeof(this->elements) );
     }

    return *this;
}

Matrix& Matrix::operator +(const Matrix& right) {
    // first, make sure matrices can be added. if not, return original matrix
    if (this->row != right.row || this->column != right.column){
        cout << "Matrix sizes do not match.";
        return (*this);
    }

    Matrix sum(row, column);
    for (int i=0; i<this->row; i++){
        for (int j=0; j<this->column; j++){
            sum.elements[i][j] = this->elements[i][j] + right.elements[i][j];

        }
    }
    return sum;
}

Моя основная программа просто создает матрицу A с матрицей 4x4, равной 1 с, и matrixB с матрицей 4x4, равной 2 с, и добавляет их, где сумма = matrixC, что должно привести к матрице 4x4, равной 3s. Тем не менее, это мой результат:

matrixA
1   1   1   1   
1   1   1   1   
1   1   1   1   
1   1   1   1   

matrixB
2   2   2   2   
2   2   2   2   
2   2   2   2   
2   2   2   2   

matrixC before addition
0   0   0   0   
0   0   0   0   
0   0   0   0   
0   0   0   0   

matrixC after addition
1.0147e-316 3   3   3   
1.0147e-316 3   3   3   
1.0147e-316 3   3   3   
1.0147e-316 3   3   3

Когда я тестировал его, чтобы увидеть, какие элементы передаются в 'right' в функции '=', я заметил данные, аналогичные первому столбцу matrixC. Я полагаю, что я не понимаю передачи ссылок, вовлеченных здесь полностью, поэтому я хотел бы некоторую помощь в этом.

Answer 1

Ваш оператор присваивания неисправен (и логически ошибочен).

Он логически ошибочен, потому что присваивание, подобное m1 = m2, должно привести к m1 == m2, независимо от того, каким было предыдущее состояние m1.Ваша реализация пытается копировать только тогда, когда они имеют одинаковый размер.Вы должны изменить размер матрицы в левой части задания.

Ваш оператор также неправильно использует memcpy.memcpy(dest, src, count) скопирует непрерывные данные с адреса src на адрес dest.Адреса, которые вы предоставляете, являются адресами указателей на указатели на int , , а не указателями на непрерывный блок int с.Здесь есть два уровня косвенности, о которых memcpy просто понятия не имеет.Кроме того, sizeof(this->elements) - это sizeof(int**), который будет константой, вероятно, 4 или 8 байтов, что также должно показаться неправильным.Чтобы это исправить, я бы просто использовал старый набор циклов for.

Matrix& Matrix::operator=(const Matrix& other){
    // attempt to acquire memory before modifying (for exception safety)
    int** temp = new int*[other.row];
    for (int i = 0; i < other.row; ++i){
        temp[i] = new int[other.col];
    }

    // cleanup
    for (int i = 0; i < row; ++i){
        delete[] elements[i];
    }
    delete[] elements;

    // copy data
    elements = temp;
    row = other.row;
    col = other.col;
    for (int i = 0; i < row; ++i){
        for (int j = 0; j < col; ++j){
            elements[i][j] = other.elements[i][j];
        }
    }

    return *this
}

Еще одно важное замечание: ваш operator+ возвращает ссылку на локальную переменную .Это очень плохо, но C ++ не защищает вас от этого .Вы должны просто вернуть sum по значению.

В современном C ++ мы обычно предпочитаем использовать более безопасные альтернативы ручному управлению памятью, такие как std::vector<std::vector<int>>, которые имеют большое значение для обеспечения правильного использования и сохранения вас.от себя.

Answer 2

Хм, мой компилятор CLang предупреждает меня, что operator +(const Matrix& right) возвращает ссылку на временное значение, которое является неопределенным поведением.

Вы должны вернуть простой объект, а не ссылку:

Matrix operator=(const Matrix& right);  // assignment operator

Answer 3

Matrix& Matrix::operator +(const Matrix& right) {
    // ...

    Matrix sum(row, column);
    // ...
    return sum;
}

Вы возвращаете ссылку на автоматический локальный (sum).

Здесь вы можете увидеть канонические формы арифметических операторов здесь , и сложение должно выглядеть как

Matrix Matrix::operator+(const Matrix &b) const;

Существуют и другие проблемы, некоторые из которых упоминаются в других ответах, но единственное, что надежно создает ваш текущий operator+, - это неопределенное поведение.

Answer 4

Подсказка: если вы сохраните матрицу как один массив double m[rows * columns], реализация будет намного проще.

Индексирование [row][column] равно m[row * columns + column].

Суммирование - это просто суммирование двух массивов поэлементно (по существу, векторная сумма).