0

I am new to c++ and I have to create a 2D array using pointers. I have completed the creation of the array but I cannot run anything after the initial creation of the matrix due to a "Segmentation fault". I believe this is from my assignment operator, but I'm not sure. This could be from the copy constructor, but because we have to use pointers I am lost. Any help would be appreciated.

#ifndef My_Matrix_H
#define My_matrix_H
#include "My_matrix.h"
#include <stdexcept>

My_matrix::My_matrix() //contructor
{
n = 0;
m = 0;
ptr = NULL;
}

My_matrix::My_matrix(int n1, int m1) //creation of the matrix
{

ptr = new int*[n1];
for (int i = 0; i < n1; i++)
{
    ptr[i] = new int[m1];
    for (int j = 0; j < m1; j++)
    {
        ptr[i][j] = 0;
    }
}

}

My_matrix::My_matrix(const My_matrix& mat) //copy constructor
{

    n = mat.n;
    m = mat.m;
    ptr = new int*[n];
    for (int i = 0; i < n; i++)
    {
        ptr[i] = new int[m];
        for (int j = 0; j < m; j++)
        {
            ptr[i][j] = mat.ptr[i][j];
        }
    }

}

My_matrix::~My_matrix() //destructor
{
    for (int i = 0; i < n; i++)
    {
        delete[] ptr[i];
    }
    delete[] ptr;
}

My_matrix& My_matrix::operator=(const My_matrix& mat) //assignmemt operator
{

    n = mat.n;
    m = mat.m;
    ptr = new int*[n];
    cout << "1" << endl;
    for (int i = 0; i < n; i++)
    {
        cout << "2" << endl;
        ptr[i] = new int[m];
        for (int j = 0; j < m; j++)
        {
            cout << "3" << endl;
            ptr[i][j] = mat.ptr[i][j];
        }
    }

}

void My_matrix::elemset(int i, int j, int num) //puts stuff into matrix
{
ptr[i][j] = num;
}
}#endif
Ulrich Eckhardt
  • 16,572
  • 3
  • 28
  • 55
Alowishious
  • 83
  • 1
  • 10
  • why do you allocate memory in `operator=`? You already did that in the constructor and just need to copy the values – 463035818_is_not_an_ai Feb 01 '18 at 18:58
  • @tobi303 It seems to be accounting for assignment between matrices with different sizes. – François Andrieux Feb 01 '18 at 19:00
  • Note that you are not releasing your previously allocated arrays whenever you assign, which is allocating new arrays. – François Andrieux Feb 01 '18 at 19:00
  • 1
    Have you checked which line is causing the segfault with a debugger? – François Andrieux Feb 01 '18 at 19:01
  • 1
    I find it suspicious that you are using an include guard in the implementation file, and that it's name matches what you would expect for the header you include. – François Andrieux Feb 01 '18 at 19:02
  • @FrançoisAndrieux now i get it. when i see too many news my brain sometimes stops working ;) – 463035818_is_not_an_ai Feb 01 '18 at 19:02
  • @FrançoisAndrieux would I insert a delete[] in the copy constuctor somewhere? This is a project and pointers are something I am fuzzy on – Alowishious Feb 01 '18 at 19:05
  • @Alowishious -- Please use copy / swap: `My_matrix& My_matrix::operator=(const My_matrix& mat) { My_matrix temp(mat); std::swap(n,temp.n);std::swap(m, temp.m);std::swap(ptr, temp.ptr); return *this;}`. [More here](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) -- there is no need to write assignment operators, basically duplicating the copy constructor. – PaulMcKenzie Feb 01 '18 at 19:06
  • Use a std::vector of size row*col instead of new[]. –  Feb 01 '18 at 19:31

2 Answers2

1

[1] In My_matrix(int,int) members n and m are not set (this is cause of all troubles, if you call any of these members (copy constructor, assignment operator or destructor) after creating object by My_matrix(int,int) without setting n,m, your application must crash).

[2] My_matrix::operator= should return *this.

[3] In My_matrix::operator= you should delete previous allocated data.

rafix07
  • 20,001
  • 3
  • 20
  • 33
0

For the answer. A copy/swap was most effective, as given by @PaulMcKenzie.

My_matrix& My_matrix::operator=(const My_matrix& mat) 

{ 

 My_matrix temp(mat); 
 std::swap(n,temp.n);
 std::swap(m, temp.m);
 std::swap(ptr, temp.ptr); 
 return *this;

}
Alowishious
  • 83
  • 1
  • 10