0

I'm trying to do a Matrix class using C++ Vector, but i don't know why the inside of "Matrix result" inside my function isn't passed to my object but it remain enclosed inside the function.

for semplicity so far I've tryed only to do an "addition function" among two Matrices.

I have tryied to work with pointer but in this way (according to my knowledgs) i cant call my funtion to an object in this wise:

foo.function1(bar1).function2(bar2);

but working with pointer i have to call function in this manner:

foo.function1(bar1);
foo.function2(bar2);
//and so on..

this is my header file:

#include <iostream>
#include <vector>

using namespace std;


class Matrix 
{
    public:

        Matrix (int height, int width);

        Matrix add(Matrix m);

        Matrix applyFunction(double (*function)(double));

        void print();


    private:

        vector<vector<double> > matrix;

        int height;
        int width;
};

this is the .cpp file:

Matrix::Matrix(int height, int width)
{
    this->height = height;
    this->width = width;

    this->matrix = vector<vector<double> >(this->height, vector<double>(this->width));
}

Matrix Matrix::add(Matrix m)
{

    Matrix result(this->height, this->width);

    if (m.height== this->height&& m.width== this->width)
    {

        for (int i = 0; i < this->height; i++)
        {
            for (int j = 0; j < this->width; j++)
            {

                result.matrix[i][j] = this->matrix[i][j] + m.matrix[i][j];
            }

            return result;
        }
    }
    else
    {

        cout << "Impossible to do addition, matrices doesn't have the same dimension" << endl;
        return result;
    }
}

Matrix Matrix::applyFunction(double(*function)(double))
{

    Matrix result(this->height, this->width);

    for (int i = 0; i < this->height; i++)
    {
        for (int j = 0; j < this->width; j++)
        {

            result.matrix[i][j] = (*function)(this->matrix[i][j]);
        }
    }

    return result;
}

void Matrix::print()
{

    for (int i = 0; i < this->height; i++)
    {
        for (int j = 0; j < this->width; j++)
        {

            cout << this->matrix[i][j] << " ";
        }

        cout << endl;
    }

    cout << endl;
}

the output should be the addition beetwen A B 2x2:

x1 x2

x3 x4

but computer show only zeros.

  • What does your `main` look like? Can we get a [mre]? – NathanOliver Aug 01 '19 at 13:10
  • About [using namespace std](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice)... – Aconcagua Aug 01 '19 at 13:13
  • Outputting in operators or equivalent functions (actually, I'd rename `add` to `operator+`...) is a rather bad idea. You impose that on anyone using your code, but someone might not want it, have a different text or another language. Returning some dummy value as result is questionable, too, better throw an exception. – Aconcagua Aug 01 '19 at 13:59

2 Answers2

0

Your member functions all return a new object (they return "by value").

From your usage of chaining, it seems like you actually want to modify the object and return *this by reference.

Otherwise you'll need something like:

auto bar2 = foo.function1(bar1);
auto bar3 = foo.function2(bar2);
// etc

There are no pointers here at present.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
  • but it isn't elegant there's another way? –  Aug 01 '19 at 13:13
  • 1
    That depends on what you want to do instead. You didn't say. I suggest looking at common implementations of arithmetic operator overloads in other projects, as they've had the same decisions to make. – Lightness Races in Orbit Aug 01 '19 at 13:15
  • @Phoenix Sidenote: You don't need *all* those temporaries, but you need to assign the *final* result to whatever appears appropriate to you, e. g. `foo = foo.function1(bar1).function2(bar2).function3(bar3);`, assuming you don't need the original value of `foo` any more... – Aconcagua Aug 01 '19 at 14:04
0

There are two variants how you can implement your add

Matrix add(Matrix m)
{
    // optimisation: you don't need separate result, m already IS a copy!
    // so you can just calculate:
    ...
    {
        m.matrix[i][j] += this->matrix[i][j];
    }

    return m;
}

or:

Matrix& add(Matrix const& m)
//                    ^ accept const reference to avoid unnecessary copy
//    ^ returning reference(!)
{
    ...
    {
        // modifies itself!
        this->matrix[i][j] += m.matrix[i][j];
    }

    return *this; // <- (!)
}

This allows now to do:

Matrix m0, m1, m2;
m0.add(m1).add(m2);
// m0 now contains the result, original value is lost (!)

So you don't need the final assignment as in first variant:

m0 = m0.add(m1).add(m2);
// or assign to a new variable, if you want to retain m0's original values

which is what you lacked in your question (thus you did not get the desired result).

Maybe you want to have both variants, and you might rename one of. But there's a nice feature in C++ that you might like even better: Operator overloading. Consider ordinary int:

int n0, n1;
n0 += n1;
int n2 = n0 + n1;

Well, suppose you know what's going on. And if you could do exactly the same with your matrices? Actually, you can! You need to do is overloading the operators:

Matrix& operator+=(Matrix const& m)
{
    // identical to second variant of add above!
}

Matrix operator+(Matrix m) // again: the copy!
{
    // now implement in terms of operator+=:
    return m += *this;
}

Yes, now you can do:

Matrix m0, m1, m2;
m0 += m1 += m2;
m2 = m1 + m0;

Alternatively (and I'd prefer it) you can implement the second operator (operator+) as free standing function as well:

// defined OUTSIDE Matrix class!
Matrix operator+(Matrix first, Matrix const& second)
{
    return first += second;
}

Finally: If dimensions don't match, better than returning some dummy matrix would be throwing some exception; std::domain_error might be a candidate for, or you define your own exception, something like SizeMismatch. And please don't output anything to console or elsewhere in such operators, this is not what anybody would expect from them, additionally, you impose console output to others who might consider it inappropriate (perhaps they want output in another language?).

Aconcagua
  • 24,880
  • 4
  • 34
  • 59