Transpose method for Matrix Class

Question

I am currently working on a matrix class and I can't decide which method is best to process a matrix transpose.

At first I did the classical way:

Matrix Matrix::Transpose()
{
    Matrix M(m_cols, m_rows);

    for(int i=0; i < m_rows; ++i)
    {
        for(int j=0; j < m_cols; ++j)
        {
            M(j,i) = this->m_matrix[i][j];
        }
    }

    return M;
}

But on a second thought I was wondering if this way was better in term of memory management:

Matrix& Matrix::Transpose()
{
    std::unique_ptr<Matrix> M(new Matrix(*this));

    m_matrix.resize(m_cols);
    for(unsigned long i = 0; i < m_matrix.size(); ++i)
    {
        m_matrix[i].resize(m_rows, 0.0);
    }
    m_rows = M->Get_Cols();
    m_cols = M->Get_Rows();

    for(unsigned long i=0; i < M->Get_Rows(); ++i)
    {
        for(unsigned long j=0; j < M->Get_Cols(); ++j)
        {
            this->m_matrix[j][i] = (*M)(i,j);
        }
    }

    return *this;
}

Now both methods work, but I'm quite new to the "memory management" side of C++ and I can't really tell which one is better in terms of "good practice"...

These two are not equivalent, notice how the first version can be made `const` as it does not transform the input matrix. The latter one however can work in-place (without allocating extra memory). — orhtej2, Sep 02 '18 at 17:44
To begin with, the two functions you show do different things. First first one doesn't modify `this` but the other does. The second one also does a lot of extra work that isn't needed if you want to modify `this`. — Some programmer dude, Sep 02 '18 at 17:44
Check this out: https://stackoverflow.com/questions/16737298/what-is-the-fastest-way-to-transpose-a-matrix-in-c — arorias, Sep 02 '18 at 17:49

score 1 · Accepted Answer · answered Sep 02 '18 at 18:23

Your first construction is preferable for several reasons, which I've discussed below. FYI: You may find return value optimization interesting as well.

Keep It Simple

The second solution is needlessly complex. You've replaced what's essentially 4 lines of code with 9 lines of code. You've also introduced another for loop and the use of heap memory. The second solution is slower, almost by definition, because you're doing more work.

Use the Stack

When working with temporary data structures, as you've done in the second example, you should prefer stack memory. Introducing additional memory allocation introduces overhead you don't need.

Avoid Surprising Behavior + Side-Effects

Your second construction modifies your instance, which for this type of transformation could be surprising.

Hints

Some additional hints for working with C++.

Don't specify this-> unless necessary - it is implied
Mark your methods const where possible - see the Cpp Core Guidelines
Make use of private implementation within your own class

Regarding the third point above, the private keyword applies at a type level, not the instance. Take a look at the code below. What do you think the output is? Notice that within our own class method, we can refer to the private variables of another instance. The action method is marked const - so we know it won't affect our instance, but the parameter is a mutable instance that we can modify.

#include <iostream>
using namespace std;

class Test
{
public:
    Test() = default;
    ~Test() = default;
    int x;
    int y;
    void setZ()
    {
        m_z = x * y;
    }
    void action(Test& other) const
    {
        other.m_z = m_z;
    }
    int z() const { return m_z; }
private:
    int m_z;
};

int main()
{
    Test a;
    Test b;
    a.x = 5;
    a.y = 3;
    a.setZ();
    a.action(b);
    cout << b.z() << endl;

    return 0;
}

Transpose method for Matrix Class

1 Answers1

Keep It Simple

Use the Stack

Avoid Surprising Behavior + Side-Effects

Hints