The issue I am having is how to get the correct number columns to go through for the inner most loop of K. An example is a 2x3 matrix and a 3x2 matrix being multiplied. The result should be a 2x2 matrix, but currently I dont know how to send the value of 2 to the operator overloaded function. It should be int k = 0; k < columns of first matrix;k
Matrix::Matrix(int row, int col)
{
rows = row;
cols = col;
cx = (float**)malloc(rows * sizeof(float*)); //initialize pointer to pointer matrix
for (int i = 0; i < rows; i )
*(cx i) = (float*)malloc(cols * sizeof(float));
}
Matrix Matrix::operator * (Matrix dx)
{
Matrix mult(rows, cols);
for (int i = 0; i < rows; i )
{
for (int j = 0; j < cols; j )
{
mult.cx[i][j] = 0;
for (int k = 0; k < ?;k ) //?????????????
{
mult.cx[i][j] = cx[i][k] * dx.cx[k][j];
}
}
}
mult.print();
return mult;
//calling
Matrix mult(rowA, colB);
mult = mat1 * mat2;
}
CodePudding user response:
Linear algebra rules say the result should have dimensions rows x dx.cols
Matrix Matrix::operator * (Matrix dx)
{
Matrix mult(rows, dx.cols);
for (int i = 0; i < rows; i )
{
for (int j = 0; j < cols; j )
{
mult.cx[i][j] = 0;
for (int k = 0; k < cols;k ) //?????????????
{
mult.cx[i][j] = cx[i][k] * dx.cx[k][j];
}
}
}
mult.print();
return mult;
CodePudding user response:
A few random hints:
- Your code is basically C; it doesn’t use (e.g.) important memory-safety features from C . (Operator overloading is the only C -like feature in use.) I suggest that you take advantage of C a bit more.
- Strictly avoid
malloc()
in C . Usestd::make_unique(...)
or, if there is no other way, a rawnew
operator. (BTW, there is always another way.) In the latter case, make sure there is a destructor with adelete
ordelete[]
. The use ofmalloc()
in your snippet smells like a memory leak. - What can be
const
should beconst
. Initialize as many class members as possible in the constructor’s initializer list and make themconst
if appropriate. (For example,Matrix
dimensions don’t change and should beconst
.) - When writing a container-like class (which a
Matrix
may be, in a sense), don’t restrict it to a single data type; your future self will thank you. (What if you need adouble
instead of afloat
? Is it going to be a one-liner edit or an all-nighter spent searching where a forgottenfloat
eats away your precision?)
Here’s a quick and dirty runnable example showing matrix multiplication:
#include <cstddef>
#include <iomanip>
#include <iostream>
#include <memory>
namespace matrix {
using std::size_t;
template<typename Element>
class Matrix {
class Accessor {
public:
Accessor(const Matrix& mat, size_t m) : data_(&mat.data_[m * mat.n_]) {}
Element& operator [](size_t n) { return data_[n]; }
const Element& operator [](size_t n) const { return data_[n]; }
private:
Element *const data_;
};
public:
Matrix(size_t m, size_t n) : m_(m), n_(n),
data_(std::make_unique<Element[]>(m * n)) {}
Matrix(Matrix &&rv) : m_(rv.m_), n_(rv.n_), data_(std::move(rv.data_)) {}
Matrix operator *(const Matrix& right) {
Matrix result(m_, right.n_);
for (size_t i = 0; i < m_; i)
for (size_t j = 0; j < right.n_; j) {
result[i][j] = Element{};
for (size_t k = 0; k < n_; k) result[i][j] =
(*this)[i][k] * right[k][j];
}
return result;
}
Accessor operator [](size_t m) { return Accessor(*this, m); }
const Accessor operator [](size_t m) const { return Accessor(*this, m); }
size_t m() const { return m_; }
size_t n() const { return n_; }
private:
const size_t m_;
const size_t n_;
std::unique_ptr<Element[]> data_;
};
template<typename Element>
std::ostream& operator <<(std::ostream &out, const Matrix<Element> &mat) {
for (size_t i = 0; i < mat.m(); i) {
for (size_t j = 0; j < mat.n(); j) out << std::setw(4) << mat[i][j];
out << std::endl;
}
return out;
}
} // namespace matrix
int main() {
matrix::Matrix<int> m22{2, 2};
m22[0][0] = 0; // TODO: std::initializer_list
m22[0][1] = 1;
m22[1][0] = 2;
m22[1][1] = 3;
matrix::Matrix<int> m23{2, 3};
m23[0][0] = 0; // TODO: std::initializer_list
m23[0][1] = 1;
m23[0][2] = 2;
m23[1][0] = 3;
m23[1][1] = 4;
m23[1][2] = 5;
matrix::Matrix<int> m32{3, 2};
m32[0][0] = 5; // TODO: std::initializer_list
m32[0][1] = 4;
m32[1][0] = 3;
m32[1][1] = 2;
m32[2][0] = 1;
m32[2][1] = 0;
std::cout << "Original:\n\n";
std::cout << m22 << std::endl << m23 << std::endl << m32 << std::endl;
std::cout << "Multiplied:\n\n";
std::cout << m22 * m22 << std::endl
<< m22 * m23 << std::endl
<< m32 * m22 << std::endl
<< m23 * m32 << std::endl
<< m32 * m23 << std::endl;
}
Possible improvements and other recommendations:
- Add consistency checks.
throw
, for example, astd::invalid_argument
when dimensions don’t match on multiplication, i.e. whenm_ != right.n_
, and astd::range_error
when theoperator []
gets an out-of-bounds argument. (The checks may be optional, activated (e.g.) for debugging using anif constexpr
.) - Use a
std::initializer_list
or the like for initialization, so that you can have (e.g.) aconst Matrix
initialized in-line. - Always check your code using
valgrind
. (Tip: Buliding with-g
letsvalgrind
print also the line numbers where something wrong happened (or where a relevant preceding (de)allocation had happened).) - The code could me made shorter and more elegant (not necessarily more efficient; compiler optimizations are magic nowadays) by not using
operator []
everywhere and having some fun with pointer arithmetics instead. - Make the type system better, so that (e.g.)
Matrix
instances with different types can play well with each other. Perhaps aMatrix<int>
multiplied by aMatrix<double>
could yield aMatrix<double>
etc. One could also support multiplication between a scalar value and aMatrix
. Or between aMatrix
and astd::array
,std::vector
etc.