Incorrect function result [требует правки]

203
23 марта 2017, 21:54

I overloaded operator * for my custom matrix, result inside function is correct, but outside function value at [0][0] is always 0.

matrix.cpp

#include "matrix.h"
Matrix::Matrix()
{
    std::cout << "__Default" << std::endl;
    this->width = 0;
    this->height = 0;
    this->values = 0;
}
Matrix::Matrix(int width, int height)
{
    std::cout << "__Constructor" << std::endl;
    this->width = width;
    this->height = height;
    this->values = new float [width * height];
    for (int i = 0; i < width; i++)
    {
        for (int j = 0; j < height; j++)
        {
            this->values[i * height + j] = (i == j ? 1 : 0);
        }
    }
}
Matrix::Matrix(const Matrix &obj)
{
    std::cout << "__Copy" << std::endl;
    this->width = obj.width;
    this->height = obj.height;
    if (obj.width == 0 || obj.height == 0)
    {
        this->values = 0;
    }
    else
    {
        this->values = new float [width * height];
        for (int i = 0; i < obj.width; i++)
        {
            for (int j = 0; j < obj.height; j++)
            {
                this->values[i * height + j] = obj.values[i * height + j];
            }
        }
    }
}
Matrix::~Matrix()
{
    std::cout << "__Destructor" << std::endl;
    if (values == 0)
        return;
    delete values;
}
float* Matrix::operator[](int i)
{
    return values + i * height;
}
const float* Matrix::operator[](int i) const
{
    return values + i * height;
}
const Matrix& operator*(const Matrix& left, const Matrix& right)
{
    std::cout << "__operator *" << std::endl;
    if (left.width != right.height)
        return Matrix();
    Matrix result = Matrix(right.width, left.height);
    for (int i = 0; i < result.width; i++)
    {
        for (int j = 0; j < result.height; j++)
        {
            result[i][j] = 0;
            for (int k = 0; k < left.width; k++)
            {
                result[i][j] += left[k][j] * right[i][k];
            }
        }
    }
    return result;
}
int Matrix::getWidth()
{
    return width;
}
int Matrix::getHeight()
{
    return height;
}
void Matrix::print()
{
    for (int j = 0; j < height; j++)
    {
        for (int i = 0; i < width; i++)
        {
            printf("%.2f%c", (*this)[i][j], (i != width - 1 ? ' ' : '\n'));
        }
    }
}
void Matrix::print() const
{
    for (int j = 0; j < height; j++)
    {
        for (int i = 0; i < width; i++)
        {
            printf("%.2f%c", (*this)[i][j], (i != width - 1 ? ' ' : '\n'));
        }
    }
}
EDIT

Добавил в код все правки

#include "matrix.h"
Matrix::Matrix()
{
    std::cout << "__Default" << std::endl;
    this->width = 0;
    this->height = 0;
    this->values = 0;
}
Matrix::Matrix(unsigned int width, unsigned int height)
{
    std::cout << "__Constructor" << std::endl;
    this->width = width;
    this->height = height;
    this->values = new float [width * height];
    for (int j = 0; j < height; j++)
    {
        for (int i = 0; i < width; i++)
        {
            this->values[i * height + j] = (i == j ? 1 : 0);
        }
    }
}
Matrix::Matrix(const Matrix &obj)
{
    std::cout << "__Copy" << std::endl;
    this->width = obj.width;
    this->height = obj.height;
    if (obj.width == 0 || obj.height == 0)
    {
        this->values = 0;
    }
    else
    {
        this->values = new float [width * height];
        for (int i = 0; i < obj.width; i++)
        {
            for (int j = 0; j < obj.height; j++)
            {
                this->values[i * height + j] = obj.values[i * height + j];
            }
        }
    }
}
Matrix::~Matrix()
{
    std::cout << "__Destructor" << std::endl;
    if (values == 0)
        return;
    delete [] values;
}
float* Matrix::operator[](int i)
{
    return values + i * height;
}
const float* Matrix::operator[](int i) const
{
    return values + i * height;
}
const Matrix operator*(const Matrix& left, const Matrix& right)
{
    std::cout << "__operator *" << std::endl;
    if (left.width != right.height)
        return Matrix();
    Matrix result = Matrix(right.width, left.height);
    for (int i = 0; i < result.width; i++)
    {
        for (int j = 0; j < result.height; j++)
        {
            result[i][j] = 0;
            for (int k = 0; k < left.width; k++)
            {
                result[i][j] += left[k][j] * right[i][k];
            }
        }
    }
    return result;
}
int Matrix::getWidth()
{
    return width;
}
int Matrix::getHeight()
{
    return height;
}
void Matrix::print() const
{
    for (int j = 0; j < height; j++)
    {
        for (int i = 0; i < width; i++)
        {
            printf("%.2f%c", (*this)[i][j], (i != width - 1 ? ' ' : '\n'));
        }
    }
}

Вывод при:

Matrix matrix = Matrix(4,4);
matrix[1][0] = 2;
matrix.print();
Matrix translate = Matrix(4,4);
translate[3][1] = 3;
translate.print();
Matrix result = matrix * translate;
result.print();
result = translate * matrix;
result.print();

Answer 1

Данное определение оператора некорректное, так как он возвращает ссылку на локальную переменную, которая перестанет существовать после выхода из оператора

const Matrix& operator*(const Matrix& left, const Matrix& right)
           ^^^
{
    std::cout << "__operator *" << std::endl;
    if (left.width != right.height)
        return Matrix();
    Matrix result = Matrix(right.width, left.height);
    //...
    return result;
}

Возвращайте не ссылку, а временный объект, то есть объявите оператор как

const Matrix operator*(const Matrix& left, const Matrix& right);

К тому же у вас неразбериха со значениями членов данных width и height. Например, в конструкторе используется такой порядок циклов

for (int i = 0; i < width; i++)
{
    for (int j = 0; j < height; j++)
    {
        this->values[i * height + j] = (i == j ? 1 : 0);
    }
}

а в функции print уже другой порядок циклов

void Matrix::print()
{
    for (int j = 0; j < height; j++)
    {
        for (int i = 0; i < width; i++)
        {
            printf("%.2f%c", (*this)[i][j], (i != width - 1 ? ' ' : '\n'));
        }
    }
}

Так что совершенно не понятно, чему соответствуют width и height, то есть что соответствует числу колонок в матрице, а что числу строк в матрице,

Кроме того, нет совершенно никакой необходимости объявлять две функции print. Оставьте объявление только константной функции.

Также совершенно не понятно, почему эти члены класса имеют знаковый целочисленный тип. По крайней мере вы тогда обязаны проверять, что в качестве аргументов не переданы отрицательные числа конструктору.

В деструкторе вы обязаны использовать оператор delete [] вместо оператора delete

Matrix::~Matrix()
{
    std::cout << "__Destructor" << std::endl;
    if (values == 0)
        return;
    delete [] values;
    ^^^^^^^^^^
}
READ ALSO
Сумма задача на жадный алгоритм!

Сумма задача на жадный алгоритм!

Всем привет есть задача! Я её решил перебором, но эту задачу нужно решить жадным алгоритмомПожалуйста помогите мне решить эту задачу! И также...

330
Использование QList&lt;QObjectDerived*&gt; как модель

Использование QList<QObjectDerived*> как модель

Есть класс, производный от QObject

223
Не могу решить задачку. Операторы ввода/вывода, Параметры вывода

Не могу решить задачку. Операторы ввода/вывода, Параметры вывода

Подскажите, что не такПо условию вроде правильно, насколько я понял, но проверку не проходит

328
Как считывать структуру из файла?

Как считывать структуру из файла?

Допустим, у меня есть структура

191