-2
\$\begingroup\$

I would like to make a Matrix class. Can any of you help me about this class?

There are two ways:

#include <iostream>

class Matrix {

private:
    double** array;
    int row, column;

public:

    Matrix(int m, int n) {
        this->row=m;
        this->column=n;
        this->array = new double*[m];
        for (int i = 0; i < n; i++){
            this->array[i] = new double[n];
            std::fill(array[i],array[i]+n,0.0);
        }
    }

    Matrix(Matrix &matrix){
        this->array=matrix.array;
        this->row=matrix.row;
        this->column=matrix.column;

    }
    Matrix& operator=(const Matrix& rhs ){
        this->row=rhs.row;
        this->column=rhs.column;
        for(int i=0;i<row;++i)
            for(int j=0;j<column;++j)
                this->array[i][j]=rhs.array[i][j];
    }

    ~Matrix() {
        for (int i = 0; i < this->row; i++)
            delete[] this->array[i];
        delete[] this->array;
    }

   void print(){
        for(int i=0;i<this->row;++i){
            for(int j=0;j<this->column;++j)
                printf("%f\t",this->array[i][j]);
            printf("\n");
        }

    }
    double * operator [](int m){
        return this->array[m];
    }
};
int main(int argc, const char * argv[]) {
    Matrix A(2,2);
    A[0][1]=1;
    A[0][0]=0;
    A[1][0]=1;
    A[1][1]=1;
    A.print();
    return 0;
  }

The other one using vector:

#include <iostream>
#include <vector>

using namespace std;

class Matrix {

private:
vector< vector<double> > array;
int row, column;

public:

Matrix(int m, int n) {
    row = m;
    column = n;
    array = vector< vector<double> > (m, vector<double> (n, 0));
}


Matrix(Matrix &matrix){
    this->array=matrix.array;
    this->row=matrix.row;
    this->column=matrix.column;

}
void print(){
    for(int i=0;i<this->row;++i){
        for(int j=0;j<this->column;++j)
            printf("%f\t",this->array[i][j]);
        printf("\n");
    }

 }
 vector<double> & operator [](int m){
     return this->array[m];
 }
 };
\$\endgroup\$

closed as unclear what you're asking by forsvarir, Mast, Mathias Ettinger, Jamal Sep 20 '16 at 12:34

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Unclear what you're asking is what we say when you don't describe what the code does. What kind of numerical algebra are you doing with your matrices? \$\endgroup\$ – mdfst13 Sep 21 '16 at 18:39
1
\$\begingroup\$

You can use a vector<vector<int>> instead of p, m, and n.

No need to manage memory (using new and delete) - the vector class will take care of that.

BTW, your operator= is pretty nasty.

Instead of replicating the allocated data pointed by p, it copies the value of p.

As a result, you will get two matrix objects sharing the same data.

If you change one, then you essentially change the other.

If you deallocate one (dynamically or statically at the end of the scope in which it is allocated), then the other one will hold a pointer to an unallocated memory block.

\$\endgroup\$
  • \$\begingroup\$ it is problem with the function operator=? how to implement it ? \$\endgroup\$ – Yao Jiahao Sep 21 '16 at 11:55
  • \$\begingroup\$ @YaoJiahao: More or less the same way in which you've implemented the constructor. But again - I think that you'd be much better off with vector<vector>> int instead of what you currently have. \$\endgroup\$ – barak manos Sep 21 '16 at 11:58
  • \$\begingroup\$ Can you see that I have changed my way ? I add my change in my question? This time , am I okey ? Thanks a lot. \$\endgroup\$ – Yao Jiahao Sep 21 '16 at 12:03
  • \$\begingroup\$ @YaoJiahao: Your question now looks like a chat. This is not the way to post questions here. Please fix it to be aligned with the conventions on this website - post only the relevant piece o code. Ask concrete questions (i.e., not "is it okay?") about whatever you have doubts with, so that me and others on this website will be able to refer to it. \$\endgroup\$ – barak manos Sep 21 '16 at 12:20
  • \$\begingroup\$ I know! I will change it soon! \$\endgroup\$ – Yao Jiahao Sep 21 '16 at 13:44
1
\$\begingroup\$

1)

 using namespace std;

This is a matter of taste, when used in implementation, but I personally prefer fully-qualified names (given that std is only three characters)

2)

class matrix
{
   int **p, m, n;

I'd argue that m and n would be known to people who read math books, but there's no excuse for not renaming p to something more telling.

3)

 p[i] = new int[n];
    for (int j = 0; j < n; ++j)
        p[i][j] = 0;

You can use memset or std::fill instead of loop here

4)

matrix& operator+ (const matrix& m1)
{
    return (*this += m1);
}

matrix& operator* (const matrix& m1)
{
    return (*this *= m1);
}

Semantics are kind of clunky here. A user does not not expect operator* and operator+ to mutate an existing matrix. You should return a different object here.

5)

matrix& operator= (const matrix& T)
  {
    p = T.p;
    n = T.n;
    m = T.m;

    return *this;
}

I'd also check for self-assignment here:

if(*this != matrix)
\$\endgroup\$
  • 1
    \$\begingroup\$ This is a matter of taste No, it's highly regarded as a lousy practice. It will bite you in the behind later. Don't be coy, you can say OP should drop the practice. \$\endgroup\$ – Mast Sep 20 '16 at 11:49
  • \$\begingroup\$ @Mast I agree, but sometimes it's useful when you restrict it to say a function scope. Also, while not entirely related to my point, I assume OP used ideone, that inserts it by default. \$\endgroup\$ – Vladislavs Burakovs Sep 20 '16 at 12:32
  • \$\begingroup\$ It's a bad habit promoted by many, not just ideone. Heck, even my first C++ book promotes it. \$\endgroup\$ – Mast Sep 20 '16 at 12:35

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