-1

For my midterm assignment I am required to code a matrix with all details (like identify any size matrix, add (etc.) a number to matrix, add (etc.) two matrices together, transpose, determinant, print...).

I made a class called Matrix and wrote bunch of constructors and functions and overloaded operators.

#include <iostream>
#include <string>
#include <ctime>
#include<fstream>
#include<cstdlib>
#include<iomanip>
#include <cmath>

using namespace std;

class Matrix {
private:
    int row;
    int column;
    int value;
    int** matrix;
public:
    Matrix(int ro, int col, int val);
    Matrix(int ro, int col, char type);
    ~Matrix();
    void print();
    void resize(int ro , int col);

    void operator=(const Matrix& other);
    Matrix operator+(int num)const;

};

Matrix::Matrix(int ro=10, int col=10, int val=0)
    :row(ro), column(col), value(val) 
    {
    if (row <= 0 || column <= 0) {
        cout << "invalid row or column value";
    }
    else{
        matrix = new int* [row];
        for (int i = 0; i < row; i++) {
            matrix[i] = new int[column];

        }
        for (int i = 0; i < row; i++) {
            for (int j = 0; j < column; j++) {
                matrix[i][j] = value;
            }
        }
    }
}

Matrix::Matrix(int ro, int col, char type)
    :row(ro), column(col)
    {
    if (row <= 0 || column <= 0) {
        cout << "invalid row or column value";
    }
    else {
        matrix = new int* [row];
        for (int i = 0; i < row; i++) {
            matrix[i] = new int[column];
        }

        if (type == 'e'){

            for (int i = 0; i < row; i++) {
                for (int j = 0; j < column; j++) {
                    if (i == j) {
                        value = 1;
                        matrix[i][j] = value;
                    }
                    else {
                        value = 0;
                        matrix[i][j] = value;
                    }
                }
            }
        }
        srand(time(NULL));
        if (type == 'r') {
            for (int i = 0; i < row; i++) {
                for (int j = 0; j < column; j++) {
                    value = rand() % 256;
                    matrix[i][j] =value;
                }
            }
        }
    }
}



void Matrix::operator=(const Matrix& other) {

    this->resize(other.row, other.column);
    for (int i = 0; i < row; i++) {
        for (int j = 0; j < column; j++) {
            matrix[i][j] = other.matrix[i][j]; // this line throw exception:Exception thrown at 0x00A7BCF2 in matrix.exe: 0xC0000005: Access violation reading location 0xDDDDDDDD.
        }
    }
}

Matrix Matrix::operator+(int num) const {

    Matrix model(row, column);
    for (int i = 0; i < model.row; i++) {
        for (int j = 0; j < model.column; j++) {
            model.matrix[i][j] = matrix[i][j] + num;
        }
    }
    return model;
}

void Matrix::print() {
    for (int i = 0; i < row; i++) {
        for (int j = 0; j < column; j++) {
            cout<< setw(6) << matrix[i][j]<< " " ;
        }
        cout << endl;
    }
}

Matrix::~Matrix() {
    for (int i = 0; i < row; i++) {
        delete[] matrix[i];
    }
    delete[] matrix;
}


   void Matrix::resize(int ro, int col){
    int** copymatrix;
    copymatrix = matrix;
    matrix = new int* [ro];
    for (int i = 0; i < ro; i++) {
        matrix[i] = new int[col];
}
    for (int i = 0; i < ro; i++) {
        for (int j = 0; j < col; j++) {
            if (i >= row || j >= column) {
                matrix[i][j] = 0;
            }
            else {
                matrix[i][j] = copymatrix[i][j];
            }
        }
    }
    row = ro;
    column = col;
}

int main() {

    Matrix mat1(3, 6, 2);

    cout << endl;
    mat1 = mat1 + 5;
    mat1.print();


}

I also tried this but gives me the same error

Matrix Matrix::operator=(const Matrix& other) {

    Matrix model(other.row, other.column);
    for (int i = 0; i < model.row; i++) {
        for (int j = 0; j < model.column; j++) {
            model.matrix[i][j] = other.matrix[i][j]; //exception 
        }
    }
    return model;
}

I actually have problems with operators overloading. As I write in the main function when I use operator+ overloading it returns a matrix and after that it comes to operator= overloading and there is the problem as it returns the matrix and comes to operator= it gives me the error Exception thrown at 0x00CEBCDB in matrix.exe: 0xC0000005: Access violation reading location 0xDDDDDDDD. I think I know why but I don't know how to solve it.

This code works in CodeBlocks 17.12. It throws the error in Visual Studio 2019 (v142).

JaMiT
  • 9,693
  • 2
  • 12
  • 26
ramixix
  • 33
  • 4
  • It's hard to say without [mcve], but it seems you violated [The Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). Did you try to do something like `Matrix m1; Matrix m2 = m1;`? It will not call your assignment operator. – Yksisarvinen Oct 26 '19 at 21:36
  • 2
    Add `resize` method implementation. – marcinj Oct 26 '19 at 21:52
  • It does seem to be a near certain Rule Of Three violation, and I almost finished writing my detailed answer before I caught the fact that the shown code was fake code that won't even compile (due to the lack of that mysterious `resize`() method), instead of real code; so I decided not to bother finishing what I already code. You need read the [help], and [edit] your question so that meets all requirements of a [mre]. – Sam Varshavchik Oct 26 '19 at 21:54
  • `Matrix Matrix::operator+(const Matrix& other) const` - has a missing return statement – marcinj Oct 26 '19 at 22:06
  • otherwise with some trivial resize implementation (taken from your dtor + ctor), it compiles and runs with no exception. https://coliru.stacked-crooked.com/a/dfee7ca6e5fec4a7 – marcinj Oct 26 '19 at 22:08
  • but you should certainly implement copy constructor, otherwise rule of three as mentioned is violated – marcinj Oct 26 '19 at 22:11
  • Also - this resize from my coliru sample is wrong - as it does not copy previous values - which is implied by resize method name.... well it has also other problems (like it does not update matrix row/col class fields) so @user12279525 please dont use it in your solution – marcinj Oct 26 '19 at 22:16
  • 1
    A good early step in [debugging](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/) is to simplify. Work with a copy of your project. In that copy, get rid of all the code not needed to reproduce the problem (such as all the unused operators). You could acknowledge that something was removed via comments, but take the code out of the picture. Also, identify which line is throwing the exception. A debugger should be able to tell you this; without one, diagnostic messages sent to `std::cout` can help (easier after the code is simplified). Your goal is a [mcve]. – JaMiT Oct 26 '19 at 22:51
  • i deleted some part of codes to be more understandable but by mistake i delete the resize method sorry for that – ramixix Oct 27 '19 at 15:35
  • My guess is that Visual Studio has thrown in some code to proactively detect potential errors. I cannot reproduce using gcc, possibly because the copy at the end of `operator+` is elided away. If my guess is close, adding a copy constructor so that your code follows [the Rule of Three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) should help. You might also want to consider [the copy-and-swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom/3279550). – JaMiT Nov 02 '19 at 17:37

2 Answers2

0

Your assignment operator doesn't assign new memory for the matrix:

void Matrix::operator=(const Matrix& other) 
{
    this->resize(other.row, other.column);
    for (int i = 0; i < row; i++)
    for (int j = 0; j < column; j++) 
    {
        matrix[i][j] = other.matrix[i][j];
    }
}

So, after it, you'll be accessing memory through an uninitialized pointer.

You need assignment operators, constructors that correctly allocate memory for your matrices. You need matching destructors, too. Look up "rule of 3".

Jeffrey
  • 7,814
  • 1
  • 16
  • 32
  • I think its this missing `resize` method which does `assign new memory for the matrix` – marcinj Oct 26 '19 at 22:17
  • The implementation of the `resize` method has been added to the question. It does allocate new memory for the matrix. – JaMiT Nov 02 '19 at 17:39
-1
Matrix& Matrix::operator=(const Matrix& other) {
    if (this == &other) {
        return *this;
    }
    this->resize(other.row, other.column);
    for (int i = 0; i < row; i++) {
        for (int j = 0; j < column; j++) {
            matrix[i][j] = other.matrix[i][j];
        }
    }
    return *this;
}

Matrix Matrix::operator+(int num) const {
    for (int i = 0; i < row; i++) {
        for (int j = 0; j < column; j++) {
            this->matrix[i][j] += num;
        }
    }
    return *this;
}

When Function return a matrix by finishing the scope Destructor get called and after we call assignment operator overloading on it, it can't reach the member of that matrix because it already get deleted by Destructor so it throws a exception. I wrote this before but it did not work until i realize that we should put (&) for (=) operator if we want use this operators overloading one after the other. so now it's working well

ramixix
  • 33
  • 4
  • This is "working well"? Have you tried something like `Matrix mat1(3, 6, 2); Matrix mat2 = mat1 + 5; mat1.print(); mat2.print();`? It is very unusual for the expression `mat1+5` to change the value of `mat1`. – JaMiT Nov 02 '19 at 17:26