Problem z konstruktorem kopiującym

0

Cześć,
Mam klasę przechowującą tablicę[alokowaną dynamicznie] dwuwymiarową, reprezentującą macierz.

Matrix::Matrix(int row, int col)
{
    this->row = row;
    this->col = col;
    m_matrix = new double *[this->row];
    for(auto i = 0; i < this->row; i++)
        m_matrix[i] = new double[this->col];
}


Matrix::Matrix(double ** matrix, int row, int col)
{
    this->row = row;
    this->col = col;
    m_matrix = new double *[this->row];
    for(auto i = 0; i < this->row; i++)
        m_matrix[i] = new double[this->col];
    m_matrix = matrix;
}
Matrix::Matrix(const Matrix & matrix)
{
    this->row = matrix.row;
    this->col = matrix.col;
    m_matrix = new double *[this->row];
    for(auto i = 0; i < this->row; i++)
        m_matrix[i] = new double[this->col];
    m_matrix = matrix.m_matrix;
}

Destruktor prezentuje się następująco:

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

Według mnie destruktor jest błędny. Po wyjściu z pętli for, powinno być jeszcze delete [] m_matrix.. Niestety powyższy kod działa, a ten z usunięciem m_matrix już nie - co może być przyczyną?
Dodam, że po zastosowaniu wersji z delete [] m_matrix program wyrzuca dużo syfu.

2

Dodam, że po zastosowaniu wersji z delete [] m_matrix program wyrzuca dużo syfu.<

Jakiego?

Dlaczego nie std::vector, zamiast gołych new i delete.
Poza tym można użyć jednowymiarowej tablicy z widokiem dwuwymiarowym.
https://dsp.krzaq.cc/post/176/ucze-sie-cxx-kiedy-uzywac-new-i-delete/
https://dsp.krzaq.cc/post/448/n-wymiarowy-widok-na-macierz-w-cxx/

W copy ctor należy jeszcze przekopiować dane do nowej tablicy.
Brakuje operator=.
Zastosuj się do http://en.cppreference.com/w/cpp/language/rule_of_three

2

A co robi, chyba tu jest problem?

m_matrix = matrix;

Po inicjalizacji w drugim konstruktorze:

Matrix::Matrix(double ** matrix, int row, int col)

Takie coś przetestowałem i działa:

int main()
{
  cout << "init" << endl;
  
  double ** m_matrix;
  int row = 5;
  int col = 5;
  
  m_matrix = new double *[row];
  
  for(auto i = 0; i < row; i++)
    m_matrix[i] = new double[col];

  cout << "cleanup" << endl;
    
  for(auto i = 0; i < row; i++){
    delete [] m_matrix[i];
  }
  
  delete [] m_matrix;
  
  cout << "finished" << endl;
  
  return 0;
}

1

Między innymi takiego

double free or corruption (out): 0x000000000090b760 ***

i masę innych błędów związanych z pamięcią

Dlaczego nie std::vector? Bo nie byłoby sensu tworzenia konstruktora kopiującego, a to zadanie ze studiów, tzw. C++1410. W normalnych sytuacjach new i delete już dawno przeszło u mnie do lamusa.

@jackoi: m_matrix = matrix ma przypisywać podaną w argumencie metody tablicę to składowej klasy. (double ** m_matrix - taka jest definicja składowej).

2

Tak się tego nie robi. Kopiujesz zawartość wszystkich elementów z przekazanego obiektu do
nowo alokowanej pamięci.
http://en.cppreference.com/w/cpp/language/rule_of_three

2

@Tenonymous:
Wydaje mi się, że to właśnie ta linijka psuje wszystko, najpierw tworzysz:

m_matrix = new double *[this->row];

później:

for(auto i = 0; i < this->row; i++)
        m_matrix[i] = new double[this->col];

a na końcu nadpisujesz to co wcześniej stworzyłeś jakimś random wskaźnikiem:

m_matrix = matrix;

Jak masz w definicji składowej (double ** m_matrix)
To nie powinno być:

this->m_matrix = m_matrix; 

Bo jak w kodzie użyjesz tego konstruktora z innym wskaźnikiem, to tutaj w definicji konstruktora, ten wskaźnik nadpisujesz i pojawia się wyciek pamięci. Tak to wygląda z mojej perspektywy bo nie widzę reszty kodu.

0

Człowiek to jednak się czasem potrafi zakręcić! Dziękuje za pomoc, teraz wszystko jasne ;)

Zarejestruj się i dołącz do największej społeczności programistów w Polsce.

Otrzymaj wsparcie, dziel się wiedzą i rozwijaj swoje umiejętności z najlepszymi.