0

My destructor keeps throwing me exceptions and I dont know whats causing them. I think I did everything right, can somebody help me ? This is the code where I create a Matrix which is represented with a double pointer. Running the code gives me an exception when the destructor is called. My guess is that something is wrong with the move/copy constructor or with the initializing constructor which I copied from the web.

The error from Visual Studio is:

Exception thrown: read access violation.
this->**array** was 0x111011101110112.

Matrix.h

#pragma once
#include <iostream>

class Matrix {
    size_t rows;
    size_t cols;
    int** array;
public:
    ~Matrix();
    Matrix(size_t, size_t);
    Matrix(const Matrix&);
    Matrix(Matrix&&);
    Matrix(size_t, size_t, std::initializer_list<std::initializer_list<int>>);
};

Matrix.cpp

#include "Matrix.h"

Matrix::~Matrix() {
    for (size_t i = 0; i < rows; i++) {
        delete[] array[i];
    }
    delete[] array;
}

Matrix::Matrix(size_t rows, size_t cols)
    : rows(rows), cols(cols), array(new int* [rows]) {
    for (size_t i = 0; i < rows; i++) {
        array[i] = new int[cols];
    }
    for (size_t i = 0; i < rows; i++) {
        for (size_t j = 0; j < cols; j++) {
            array[i][j] = 0;
        }
    }
    std::cout << "CONSTRUCTOR" << std::endl;
}

Matrix::Matrix(size_t rows, size_t cols, std::initializer_list<std::initializer_list<int>> list)
    : rows(rows), cols(cols), array(new int* [rows]) {
    for (size_t i = 0; i < rows; i++) {
        array[i] = new int[cols];
    }
    size_t i = 0;
    for (auto row : list) {
        size_t j = 0;
        for (auto elem : row) {
            array[i][j] = elem;
            j++;
        }
        i++;
    }
}

Matrix::Matrix(const Matrix& other)
    : rows(other.rows), cols(other.cols), array(new int* [other.rows]) {
    if (other.array != nullptr) {
        for (size_t i = 0; i < other.rows; i++) {
            array[i] = new int[other.cols];
        }
        for (size_t i = 0; i < other.rows; i++) {
            for (size_t j = 0; j < other.cols; j++) {
                array[i][j] = other.array[i][j];
            }
        }
    }
    else {
        delete[] array;
        array = nullptr;
    }
    std::cout << "COPY" << std::endl;
}

Matrix::Matrix(Matrix&& other)
    : rows(other.rows), cols(other.cols), array(other.array) {
    other.array = nullptr;
    std::cout << "MOVE" << std::endl;
}

main.cpp

#include "Matrix.h"

Matrix f(Matrix m) { return m; }

int main() {
    Matrix m(2, 2, { {1,2},{2,3} });
    Matrix k(f(m));
}

I tried making new projects numerous times with adding different stuff but I really don't know where is the problem.

  • Post/Add the complete error/exception to your question. – Jason Nov 22 '22 at 06:36
  • 2
    You forgot to reset the matrix sizes in the copy and move constructors, and the destructor's loop attempts to `delete[] nullptr[i];` – 273K Nov 22 '22 at 06:43
  • @273K Could you spoonfeed me ? I lost hours on debugging the project I really cant find the error, to exhausted. – Edmond Dantes Nov 22 '22 at 06:48
  • 1
    The fastest fix: check `array != nullptr` in the destructor. You also have a memory leak in the else branch of the copy constructor. – 273K Nov 22 '22 at 06:57
  • It's not causing your current problem but you'll need to implement/delete the assignment operators too – Alan Birtles Nov 22 '22 at 06:57
  • @273K I cant believe what I overlooked, thank you so much. – Edmond Dantes Nov 22 '22 at 07:01
  • *"My guess is that something is wrong with the move/copy constructor or with the initializing constructor which I copied from the web."* -- Guessing is for people who cannot experiment. Getting rid of the second line, `int main() { Matrix m(2, 2, { {1,2},{2,3} }); }` tests the initializing constructor in isolation. If that passes, `int main() { Matrix m(2, 2, { {1,2},{2,3} }); Matrix other(m); }` tests your copy constructor. – JaMiT Nov 22 '22 at 07:05
  • 1
    Make your code cleaner, easier to read and more performant by replacing your `int** array` with `std::vector array` and initialise it to size `rows*cols`. – bitmask Nov 22 '22 at 07:38

1 Answers1

2

Your Matrix can have nullptr array pointers. In that case your destructor exhibits UB in this line:

delete[] array[i];

As I stated in the comments, I'd recommend not using a raw pointer. You can just use an std::vector<int> with size rows*cols. It implements copy, move, assignment and destrutors for you. And you don't have a jagged matrix which is not optimal.

bitmask
  • 32,434
  • 14
  • 99
  • 159