0

I am trying to create a Matrix class using c++. So far, I have the following accomplished:

  • Creating the Matrix
  • Deleting the Matrix
  • Getting and setting values inside of the matrix

Right now, I am working on overriding all of the operators (ie., +, -, *, /) and returning matrices. I'm having a lot of issues on this, so I was wondering if anyone could help?

And I'm also having issues with copying the matrix into a new one, so any help with that code would be appreciated.

Note: I am coming from a Python background, and I know a bit of c++. I decided that while yes, creating a lot of really cool games and OOP stuff in Python is awesome and cool, I should learn c++ in order to be getting a job when I'm older.

Here's my code, I have a header containing the prototype and the class definitions, then the main one.

matrix.h

#ifndef MATRIX_H
#define MATRIX_H

/*

// These are all of the error codes
// Upon changing errorcode, Matrix should reset to null/void

*/

#define ERROR_ROW_NP 1          // Row Number cannot be non-positive
#define ERROR_COLUMN_NP 2       // Column Number cannot be non-positive
#define ERROR_ROW_I 3           // Row Index Error
#define ERROR_COLUMN_I 4        // Column Index Error
#define ERROR_RC_MISMATCH 5     // # of Rows and Columns do not match

class Matrix {
    int row;
    int column;
    int elements;

    int *RC;

  public:
    int ERRORCODE;

    Matrix (void);                  // DONE
    Matrix (int, int);              // DONE
    ~Matrix (void);                 // DONE

    void Copy (Matrix);

    int get_value (int, int);       // DONE
    void set_value (int, int, int); // DONE

    int rc_match (Matrix);          // DONE

    Matrix operator+ (Matrix);
    Matrix operator- (Matrix);

    Matrix operator* (Matrix);

    Matrix operator* (int);
    Matrix operator/ (int);

};

#endif

matrix.cpp

#include "matrix.h"

Matrix::Matrix (void) {
    ERRORCODE = 0;

    row = 1;
    column = 1;
    elements = row * column;

    RC = new int[elements];

    for (int i=0; i< elements; i++) {
        RC[i] = 0;
    }
}

Matrix::Matrix (int r, int c) {
    ERRORCODE = 0;

    row = r;
    column = c;
    elements = row * column;

    RC = new int[elements];

    for (int i=0; i< elements; i++) {
        RC[i] = 0;
    }
}

Matrix::~Matrix (void) {
    delete[] RC;
}


// Copy will copy all of the contents of the toCopy
// matrix into itself; also resets it's own rows/columns
void Matrix::Copy (Matrix toCopy) {
    row = toCopy.row;
    column = toCopy.column;
    elements = toCopy.elements;

    RC = new int[elements];

    for (int i=0; i<elements; i++) {
        RC[i] = toCopy.RC[i];
    }
}

int Matrix::get_value (int r, int c) {
    return RC[(column*r)+c];
}

void Matrix::set_value (int r, int c, int value) {
    RC[(column*r)+c] = value;
}


int Matrix::rc_match (Matrix a) {
    if (
        (row == a.row)
        &&
        (column == a.column)
        ) {
            return (1);
    }
    else {
        return (0);
    }
}


Matrix Matrix::operator+ (Matrix a) {
    if (rc_match(a)) {
        Matrix OUT(row, column);
        int z;
        for (int i=0; i < row; i++) {
            for (int j=0; j < column; j++) {
                z = OUT.get_value(i, j) + a.get_value(i, j);
                OUT.set_value(i, j, z);
            }
        }
        return OUT;
    }
    else {
        Matrix OUT(1, 1);
        OUT.ERRORCODE = ERROR_RC_MISMATCH;
        return OUT;
    }
}

main.cpp

#include <iostream>
#include "matrix.h"

int main(void) {

    Matrix a(2, 2);
    a.set_value(0, 0, 3);
    a.set_value(0, 1, 2);

    Matrix b(2, 2);
    b.set_value(0, 0, 1);
    b.set_value(0, 1, 1);
    b.set_value(1, 0, 3);
    b.set_value(1, 1, 3);

    printf("%d %d\n", a.get_value(0, 0), a.get_value(0, 1));
    printf("%d %d\n", a.get_value(1, 0), a.get_value(1, 1));
    printf("\n");

    printf("%d %d\n", b.get_value(0, 0), b.get_value(0, 1));
    printf("%d %d\n", b.get_value(1, 0), b.get_value(1, 1));

    char t[1];
    printf("Press 'Enter' to continue...");
    std::cin.getline(t, 1);
    printf("\n");

    Matrix c;
    c.Copy(a+b);

    printf("%d %d\n", c.get_value(0, 0), c.get_value(0, 1));
    printf("%d %d\n", c.get_value(1, 0), c.get_value(1, 1));

    printf("Press 'Enter' to continue...");
    std::cin.getline(t, 1);
    printf("\n");

    return (0);
}

The error I am getting upon compiling and running is this:

Debug assertion failed! ...
Expression: _BLOCK_TYPE_IS_VALID(pHead ->nBlockUse)

That pops up after hitting 'Enter'

Also, this is my first time posting, if I did anything wrong, let me know please :]

EDIT2: I got it to work! Thank you @templatetypedef!

Here's the additional code I used: (I found out that my add function was wrong too) matrix.cpp

Matrix::Matrix(const Matrix& toCopy) {
    row = toCopy.row;
    column = toCopy.column;
    elements = toCopy.elements;

    RC = new int[elements];

    for (int i=0; i<elements; i++) {
        RC[i] = toCopy.RC[i];
    }
}

Matrix Matrix::operator+ (Matrix a) {
    if (rc_match(a)) {
        Matrix OUT(row, column);
        int z;
        for (int i=0; i < row; i++) {
            for (int j=0; j < column; j++) {
                z = get_value(i, j) + a.get_value(i, j);
                OUT.set_value(i, j, z);
            }
        }
        return OUT;
    }
    else {
        Matrix OUT(1, 1);
        OUT.ERRORCODE = ERROR_RC_MISMATCH;
        return OUT;
    }
}

So for now I shall look into the assignment operator

Polosky
  • 88
  • 5
  • 13
  • It would be helpful if you could identify what line of code this error message corresponds to. In other words, which line of `main` was it on when it crashed? – Oliver Charlesworth Jul 08 '11 at 00:20
  • 2
    You're definitely missing a copy constructor, and your `Copy` function is currently taking its argument by value. – Kerrek SB Jul 08 '11 at 00:22
  • Why not simply have access through `operator()` like so: `int & operator()(size_t i, size_t j) { return RC[column * i + j]; }` and `const int & operator(size_t i, size_t j) const { return RC[column * i + j]; }` -- that'd make your code less verbose! – Kerrek SB Jul 08 '11 at 00:24
  • @Kerrek SB What exactly is that code doing? – Polosky Jul 08 '11 at 00:34
  • 1
    Off-topic, but nonetheless important comment: Matrix multiplication is associative, i.e., provided the sequence of the matrices to be multiplied is not changed, you may multiply them in whatever order you wish. However, the chosen order affects the efficiency of the overall operation, so [some sort of analysis](http://en.wikipedia.org/wiki/Matrix_chain_multiplication) has to be done in order to multiply matrices efficiently. In addition to your `Matrix` class, I would create a `LazyMatrixExpr` class representing an lazily evaluated matrix expression, which could later be cast into `Matrix`. – isekaijin Jul 08 '11 at 00:40
  • @Eduardo Leon This is interesting. I knew that Matrix multiplication is associative, but had no idea that it might have an impact on efficiency. I was just planning on porting code from my Python scripts to C++, so I'll look more into this after I get everything working correctly. – Polosky Jul 08 '11 at 00:45
  • @Hondros: Basically, in order to implement a matrix multiplication API that 1. looks and feels as if matrices were a native type, 2. is efficient, 3. is generic, you have to implement more temporary classes than there are string classes in C++. – isekaijin Jul 08 '11 at 00:48
  • @Hondros: The operators provide easy access to the matrix elements, so that you can say, `Matrix M; M(1,4) = 8; return M(2,0);` etc. None of the get/set verbosity! :-) – Kerrek SB Jul 08 '11 at 01:31
  • If you actually need to get some work done with matrices, perhaps you'll be interested in some fine libraries. [NTL](http://www.shoup.net/ntl/doc/tour-examples.html) is very nice, if you can get through the details, and it provides lots of linear algebra features. [Boost](http://www.boost.org/doc/libs/1_46_1/libs/numeric/ublas/doc/types_overview.htm) is another obvious candidate, and maybe also [Lapack++](http://math.nist.gov/lapack++/). – Kerrek SB Jul 08 '11 at 01:38
  • @Kerrek SB Yeah, I need to implement the calling operator to my class. And thanks for the libs, but I am creating my own just as a learning experience. :) – Polosky Jul 08 '11 at 01:48
  • OK, no problem. Eduardo's suggestion to make lazy evaluation classes is very important, though, do consider that for the "Day 2" phase of your learning experience :-) – Kerrek SB Jul 08 '11 at 01:54

2 Answers2

0

I believe that the problem here is that your Matrix class has a destructor, but not a copy constructor or copy assignment operator. This means that in your code for operator +, when you return a matrix, you'll get a shallow copy of the matrix rather than a deep copy. This means that the shallow copy will share a pointer to the same elements as the locally-declared matrix in operator +. When this local variable goes out of scope, its destructor will fire, reclaiming the memory for that matrix. Consequently, the returned matrix will have a pointer to deallocated memory, resulting in undefined behavior.

To fix this, try implementing a copy constructor and assignment operator for your Matrix class. As a shameless self-plug, I once wrote a guide on how to do this, which might be useful here.

Hope this helps!

templatetypedef
  • 362,284
  • 104
  • 897
  • 1,065
0

@templatetypedef and @Kerrek have identified your problem.

It can be solved trivially by using std::vector to hold your matrix elements instead of manually managing memory. Copy constructor and assignment operator will be implemented automatically by the compiler, in terms of std::vector's copy constructor and assignment operator, and you can get rid of your destructor code. As a bonus, you could then remove the Copy function along with its memory leak.

Ben Voigt
  • 277,958
  • 43
  • 419
  • 720
  • While using STL containers do solve the correctness problem, they don't solve the efficiency problem. Usually, users of math libraries expect to be able to **freely and efficiently** operate on mathematical structures as if they were native types. – isekaijin Jul 08 '11 at 01:08
  • Thank you, but I wanted to manange the memory myself, I like having full control over my program right now. Yes, that will come handy down the road though. – Polosky Jul 08 '11 at 01:08
  • @Eduardo: Correctness first, then efficiency. And a `std::vector` shouldn't be particularly less efficient than manual memory management. Lazy evaluators can be written just as easily for a matrix wrapping a `std::vector` as a matrix containing a raw pointer. – Ben Voigt Jul 08 '11 at 01:09
  • @Hondros: Is this a learning exercise? In any case, you should put memory management in a separate class that follows the [rule of five (formerly rule of three)](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). That's the C++ RAII principle -- each resource is managed by a class whose sole responsibility is safely managing it. Separate matrix computation from resource management. I would use `std::vector` to start, you can always replace it with your own version if you find a good reason to. – Ben Voigt Jul 08 '11 at 01:11
  • @Ben Voigt Somewhat of a learning exercise yes. I'll take a look at that. Seems interesting. And I was actually thinking of doing something like that anyways. – Polosky Jul 08 '11 at 01:15