Operator overloading + dynamic matrix

Question

So I have decided to restart my program everything except the + operator is working now. I get the read access violation exception

Header File

#pragma once
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>

using namespace std;

class MatrixType
{
private:
    int **matrix;
    int row;
    int col;
public:
    void setElement(int r, int c, int newvalue);
    void display();

    const MatrixType& operator=(const MatrixType& mat);
    MatrixType operator+(const MatrixType& mat) const;

    friend std::ostream & operator << (std::ostream & out, const MatrixType & mat);

    MatrixType(int r, int c);
    MatrixType(string fileName);
    MatrixType(const MatrixType& mat);
    MatrixType();
    ~MatrixType();
};

Implementation File

#include "matrixType.h"
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>

using namespace std;

void MatrixType::setElement(int r, int c, int newvalue)
{
    matrix[r][c] = newvalue;
}

void MatrixType::display()
{
    for (int i = 0; i < row; i++)
    {
        for (int r = 0; r < col; r++)
            cout << matrix[i][r] << " ";
        cout << endl;
    }
    cout << endl;
}

const MatrixType& MatrixType::operator=(const MatrixType& mat)
{
    if (row != mat.row)
    {
        cout << "The matrixes are not identical" << endl;
        return *this;
    }
    for (int i = 0; i < row; i++)
    {
        for (int r = 0; r < col; r++)
        {
            matrix[i][r] = mat.matrix[i][r];
        }
    }
    return *this;
}

MatrixType MatrixType::operator+(const MatrixType& mat) const
{
    MatrixType tempMatrix;
    if (row != mat.row)
    {
        cout << "The matrixes are not identical can not be added together" << endl;
        return *this;
    }
    else
    {
        for (int i = 0; i < mat.row; i++)
        {
            for (int r = 0; r < mat.col; r++)
            {
                tempMatrix.matrix[i][r] = mat.matrix[i][r] + matrix[i][r];
            }
        }
        return tempMatrix;
    }
}

std::ostream & operator << (std::ostream & out, const MatrixType & mat)
{
    for (int i = 0; i < mat.row; i++) {
        for (int j = 0; j < mat.col; j++) {
            out << mat.matrix[i][j] << "  ";
        }
        out << std::endl;
    }
    return out;
}

MatrixType::MatrixType(int r, int c)
{
    matrix = new int*[r];
    for (int i = 0; i < r; i++)
    {
        matrix[i] = new int[c];
    }
    row = r;  
    col = c;
    for (int i = 0; i < row; i++)
    {
        for (int s = 0; s < col; s++)
        {
            matrix[i][s] = 0;
        }
    }
}
MatrixType::MatrixType(string fileName)
{
    int r;
    int c;
    int z;
    ifstream myFile;
    myFile.open(fileName);
    myFile >> r;
    myFile >> c;
    matrix = new int*[r];
    for (int i = 0; i < r; i++)
    {
        matrix[i] = new int[c];
    }

    for (int i = 0; i < r; i++)
    {
        for (int s = 0; s < c; s++)
        {
            myFile >> z;
            matrix[i][s] = z;
        }
    }
    row = r;
    col = c;
    myFile.close();
}

MatrixType::MatrixType(const MatrixType& mat)
{
    row = mat.row;
    col = mat.col;
    matrix = new int*[row];
    for (int i = 0; i < row; i++)
    {
        matrix[i] = new int[col];
    }
    for (int i = 0; i < row; i++)
    {
        for (int s = 0; s < col; s++)
        {
            matrix[i][s] = mat.matrix[i][s];
        }
    }
}

MatrixType::MatrixType()
{
}

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

Source Code

#include "MatrixType.h"
#include <iostream>
#include <iomanip>
#include <string>
#include <fstream>

using namespace std;

int main()
{
    MatrixType M1("matrixfile1.txt");
    MatrixType M2("matrixfile2.txt");
    MatrixType M3("matrixfile3.txt");

    cout << "Matrix 1:" << endl << M1 << endl << endl;
    cout << "Matrix 2:" << endl << M2 << endl << endl;
    cout << "Matrix 3:" << endl << M3 << endl << endl;

    MatrixType M4 = M1 + M3;

    return 0;
}

So basically the goal is to check to see if the matrixes are identical otherwise you should just see a print the matrixes are not the same size. The matrixfile1,2,3 that look like

3 3

1 1 1

That show the dimensions of the array and their elements. My question is what I could do to make my operator function work better as it is currently not able to add the matrixs together yet. Everything else though once again works its just the operator that gets a read access violation.

Off topic, but your `operator=` is flawed in several ways. First, it is counter-intutitive -- an assignment's operator job is to create a copy of what is passed in. Instead, you have "business logic" rejecting the request to copy. Note that assignment operators can be invoked by the compiler, and not explicitly by you. Thus what you will wind up having in a non-trivial program is a bunch of fake copies floating around your app, and makes very hard-to-diagnose bugs. What your assignment op should be doing is what the user expects -- make `*this` now equal to the passed-in object. — PaulMcKenzie, Feb 07 '19 at 01:06
Also, it should act in concert with the copy constructor. If `MatrixType m; MatrixType m2 = m;` works in a certain way, then `MatrixType m; MatrixType m2; m = m2;` had better give the same results, else you have a really strange, IMO broken class. — PaulMcKenzie, Feb 07 '19 at 01:08
A better implementation of `operator=` would do this: `{MatrixType temp(mat); std::swap(temp.matrix,matrix); std::swap(temp.row,row); std::swap(temp.col,col); return *this;}` -- That alone would take care of everything mentioned in the previous comments. Note that this is the [copy-swap idiom](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) — PaulMcKenzie, Feb 07 '19 at 01:14
Actually, I am learning c++ right now have never even heard of a swap function, but thank you. That's why we have business logic occurring. — Derek Langer, Feb 07 '19 at 01:25
That's ok. The bottom line is that the usual functions that are provided by default by the compiler (copy/move constructor, assignment/move operator, destructor), when overridden by user-defined versions, needs to mimic what the default version would do. Having an `operator=` that runs totally counter to what it's supposed to do (replace the current object's representation with the passed-in object's representation) is not a good idea, and especially that it can be called "behind your back" by the compiler. — PaulMcKenzie, Feb 07 '19 at 01:58

score 2 · Accepted Answer · answered Feb 07 '19 at 00:57

MatrixType::MatrixType()
{
}

This doesn't initialize the members row, col, or matrix at all, so it's undefined behavior to attempt to use the current values of any of them.

Since you haven't provided a way to change the dimensions of a MatrixType at all, even using operator=, there doesn't seem to be any good use of a default constructor, and probably the MatrixType() default constructor shouldn't exist at all.

MatrixType MatrixType::operator+(const MatrixType& mat) const
{
    MatrixType tempMatrix;

Your operator+ starts out by initializing tempMatrix using the default constructor. So the dimensions of tempMatrix are...?

WAIT NEVERMIND I FIXED IT THANK YOU – Derek Langer Feb 07 '19 at 01:04 — Derek Langer, Feb 07 '19 at 01:04
@DerekLanger inside voice, please. – user4581301 Feb 07 '19 at 01:43 — user4581301, Feb 07 '19 at 01:43

Operator overloading + dynamic matrix

1 Answers1