I have a matrix class with fields like this:

template <typename T>
class Matrix
{
private:
    T **matrix = nullptr;
    int rows; 
    int cols; 

At this stage, I have written an assignment operator and a copy constructor. But firstly, there is code duplication, how can it be avoided, and secondly, they seem very similar to me, how can these methods be improved to look normal?

Matrix(const Matrix &matrix_) : rows(matrix_.rows), cols(matrix_.cols)
        {
            matrix = static_cast<T **>(new T *[rows]);

            for (int i = 0; i < rows; i  )
            {
                matrix[i] = static_cast<T *>(new T[cols]);
            }
            for (int i = 0; i < rows; i  )
            {
                for (int j = 0; j < cols; j  )
                {
                    matrix[i][j] = matrix_[i][j];
                }
            }
        }

        Matrix &operator=(const Matrix &matrix_)
        {
            if (&matrix == this)
            {
                return *this;
            }
            clean();

            rows = matrix_.rows;
            cols = matrix_.cols;

            matrix = static_cast<T **>(new T *[rows]);

            for (int i = 0; i < rows; i  )
            {
                matrix[i] = static_cast<T *>(new T[cols]);
            }
            for (int i = 0; i < rows; i  )
            {
                for (int j = 0; j < cols; j  )
                {
                    matrix[i][j] = matrix_[i][j];
                }
            }
        }

        void clean()
        {
            if (cols > 0)
            {
                for (int i = 0; i < rows; i  )
                {
                    delete[] matrix[i];
                }
            }
            if (rows > 0)
            {
                delete[] matrix;
            }
        }

According to the condition of the assignment, it is forbidden to use STL containers, I must implement the controls myself

Added move semantics

Matrix(Matrix &&other) noexcept : rows(std::move(other.rows)), cols(std::move(other.cols)), data(new T(rows * cols))
{
    other.data = nullptr;
    rows = 0;
    cols = 0;
}

Matrix &operator=(Matrix &&other) noexcept
{

    if (&other == this)
    {
        return *this;
    }

    if (rows != other.rows && cols != other.cols)
    {
        std::cout << "Error assigning matrices of different sizes" << std::endl;
        exit(-1);
    }

    clean();

    std::swap(data, other.data);
    std::swap(rows, other.rows);
    std::swap(cols, other.cols);

    return *this;
}

CodePudding user response：

Generally, if you need common code between methods, the best way is to factor out the common code into (private) helper methods. For example, you might have a method alloc_ that has the loop calling new and a free_ that calls delete. Then you could have:

Matrix(const Matrix &a) : rows(a.rows), cols(a.cols) {
    alloc_();
    copy_(a);
}
Matrix &operator=(const Matrix &a) {
    free_();
    rows = a.rows;
    cols = a.cols;
    alloc_();
    copy_(a);
}
~Matrix() { free_(); }

CodePudding user response：

Use a 1D array then. It will be much cleaner and simpler and faster than an array of pointers to arrays... You can do something like:

template <typename T>
class Matrix
{
private:
    // Note: Assuming T is a trivial type, most likely a fundamental type...

    T* data; // Flattened matrix with size = rows*cols. Be careful,
             // for really big matrices this multiplication could overflow!!!

    // You can use unsigned type since negative size is a nonsense...
    unsigned int rows;
    unsigned int cols;

public:
    Matrix(const Matrix& other) 
        : data(new T[other.rows*other.cols])
        , rows(other.rows)
        , cols(other.cols)
    {
        /// You can do this:
        /// for(int i = 0; i < rows*cols;   i)
        ///    data[i] = other.data[i];
        /// or simply call the copy assign operator:
        operator=(other);
    }

    Matrix& operator=(const Matrix& other)
    {
        // This is only for matrix with the same dimensions.
        // Delete and allocate new array if the dimensions are different. 
        // This is up to the OP if he can have differently sized matrices or not...
        // Also assert will "disappear" if NDEBUG (ie. in release) is defined.
        assert(rows == other.rows);
        assert(cols == other.cols);
        for(int i = 0; i < rows*cols;   i)
            data[i] = other.data[i];
        return *this;
    }
};

Note: You will also need to define the constructor(s) and destructor of course... I'm pretty sure you can't make it much simpler than this...