0
\$\begingroup\$

I was bothered with existing matrix allocation methods that take variable rows and columns, so I made a small class that only needs one memory allocation.


template <typename T>
class Matrix{
    T* data;
    size_t rows, columns;

public:
    Matrix(const size_t rows, const size_t columns) : rows(rows), columns(columns) {
        // One single allocation to store data
        data = new T[rows+columns + 1];
    }

    T* operator[](const size_t r) {
        return data+(r*columns);
    }
};

// Example

int main() {

    Matrix<int> mat(5,3);

    mat[2][1] = 1;

    //...

}
\$\endgroup\$
4
  • \$\begingroup\$ What aspects do you expect reviewers to go over? \$\endgroup\$
    – user228914
    Oct 22, 2020 at 8:09
  • \$\begingroup\$ Lot missing from this. Have a look at my article on vectors. You should be able to apply the same principles to Matrix allocation. lokiastari.com/series You may also need to read up on how to overload operator[] for 2 dimensional array accesses stackoverflow.com/a/1971207/14065 \$\endgroup\$ Oct 22, 2020 at 21:00
  • \$\begingroup\$ @MartinYork Very nice article on your website, ill surely learn a lot from it. \$\endgroup\$
    – user228914
    Oct 23, 2020 at 7:18
  • \$\begingroup\$ Then there's the out-of-bounds memory access (caused because the memory allocated in the constructor is the wrong amount). \$\endgroup\$ Oct 23, 2020 at 18:35

1 Answer 1

2
\$\begingroup\$
  • Since you have the new keyword, you should make a destructor and delete[].
~Matrix()
{
    delete[] data;
    data = nullptr;
}

Note that it is not necessary to set data = nullptr in the destructor, because it will be deleted as soon as the destructor is called.

  • You are missing a copy constructor which can cause problems when assigning one matrix to another

  • What happens if the number passed in [] when slicing from the matrix is out of bounds?

  • Have a glance at std::initialzer_list in C++, it will simplify assigning values to the matrix.

  • You will have to overload the arithmetic operators ( +, -, *, /) if you want to perform any kind of operations between two matrices

  • Lastly, I was a little confused to see size_t rather than std::size_t, if you have using std::size_t in your source code, you should have it here too to avoid confusion.

\$\endgroup\$
12
  • 2
    \$\begingroup\$ You're correct about the need for a destructor, copy constructor and so on, but these things can all be avoided by using std::vector<T> data. \$\endgroup\$
    – G. Sliepen
    Oct 22, 2020 at 10:53
  • 1
    \$\begingroup\$ When you see something like this in the code //... it is better not to answer and flag it for community attention because the question is off-topic, missing review context. \$\endgroup\$
    – pacmaninbw
    Oct 22, 2020 at 11:28
  • 1
    \$\begingroup\$ I agree with you people but maybe he missed his " reinventing the wheel tag " right? \$\endgroup\$
    – user228914
    Oct 22, 2020 at 12:21
  • \$\begingroup\$ @MartinYork I wasn't aware of this, can you link any site where I can read about it? I have removed that para so I hope its fine now \$\endgroup\$
    – user228914
    Oct 23, 2020 at 16:09
  • \$\begingroup\$ @AryanParekh: You can read a bunch of my code reviews here. :-) \$\endgroup\$ Oct 23, 2020 at 16:15

Not the answer you're looking for? Browse other questions tagged or ask your own question.