0

this problem is blowing my mind... My experience with C++ (really with every OO programming paradigm) is very low, but i dont find the solution on my problem... Why my destructor make me problem on copy overloaded assignment? Any help is appreciated, have a good day

matrice.h

#ifndef MATRICE_H_
#define MATRICE_H_

typedef double tipoelem;

class matrice {
  public:
    matrice(int, int, tipoelem inizializzatore = 0); /* costruttore */
    ~matrice(void);
    tipoelem leggiMatrice(int, int);
    void scriviMatrice(int, int, tipoelem);
    void prodottoScalare(tipoelem);
    matrice matriceTrasposta(void);
    matrice matriceProdotto(matrice& M);
    matrice& operator=(const matrice&);
    void rand(void);
    void stampa(void);
  private:
    int righe;
    int colonne;
    tipoelem **elementi;
};

#endif /* MATRICE_H_ */

matrice.cpp

#include "matrice.h"
#include <stdlib.h>
#include <iostream>
// costruttore
matrice::matrice(int r, int c, tipoelem inizializzatore){
  this->colonne = c;
  this->righe = r;

  // allocazione dinamica della matrice
  elementi = new tipoelem*[righe];
  for (auto i=0; i!=righe; i++)
      this->elementi[i] = new tipoelem[colonne];

  // inizializzazione degli elementi
  for (auto i=0; i!=righe; i++)
      for(auto j=0; j!=colonne; j++)
          this->elementi[i][j] = inizializzatore;
}

matrice::~matrice(void){
    for(auto j=0;j<this->colonne;++j){
        delete[] this->elementi[j];
    }
    delete[] this->elementi;
}

tipoelem matrice::leggiMatrice(int i, int j){
    return elementi[i][j];
}

void matrice::scriviMatrice(int i, int j, tipoelem scrittura){
    elementi[i][j] = scrittura;
    return;
}

void matrice::prodottoScalare(tipoelem scalare){
    for(auto i = 0; i<righe;i++)
        for(auto j = 0; j<colonne;j++)
            elementi[i][j]=elementi[i][j]*scalare;
    return;
}

matrice matrice::matriceTrasposta(void){
    matrice trasposta(colonne, righe);

    for(auto i=0; i<righe;i++)
        for(auto j=0; j<colonne;j++)
            trasposta.scriviMatrice(j,i,leggiMatrice(i,j));
    return trasposta;
}

matrice matrice::matriceProdotto(matrice& M){
    matrice prodotto(righe, colonne);

    for(auto i=0; i<righe;i++)
        for(auto j=0; j<righe;j++)
            prodotto.scriviMatrice(i,j,(matrice::leggiMatrice(i,j)*M.leggiMatrice(i,j)));
    return prodotto;
}

matrice& matrice::operator=(const matrice &m){
    if(this != &m){
        if(colonne != m.colonne || righe != m.righe){
            this->~matrice();
            this->righe = m.righe;
            this->colonne = m.colonne;
            matrice(righe,colonne);
        }
        for(auto i=0;i!=righe;i++)
            for(auto j=0;j!=colonne;j++)
                elementi[i][j] = m.elementi[i][j];
    }
    return (*this);
}

void matrice::rand(void){
    for(auto i=0; i<righe;i++)
        for(auto j=0;j<colonne;j++)
            matrice::scriviMatrice(i,j,(random() % 100));
    return;
}

void matrice::stampa(void){
    for(auto i=0; i<righe;i++){
        for(auto j=0; j<colonne;j++)
            std::cout << elementi[i][j] << " ";
    std::cout << std::endl;
    }
}

TestMatrice.cpp (for testing propose)

#include <iostream>
#include "matrice.h"

int main(void){

    matrice A(3,2), T(2,3);
    A.rand();
    std::cout <<"Stampa A" << std::endl;
    A.stampa();
    std::cout << "Stampa Trasposta T" << std::endl;
    T = A.matriceTrasposta();
    T.stampa();

    std::cout << std::endl;

    std::cout << "Stampa B" << std::endl;
    matrice B(4,4);
    B.stampa();
    std::cout << "Stampa copia t in b" << std::endl;
    B = T;
    B.stampa();

    return (0);
}

Tx

P.s.

Console output and debugging info:

Stampa A
83 86 
77 15 
93 35 
Stampa Trasposta T
83 77 93 
86 15 35 

Stampa B
0 0 0 0 
0 0 0 0 
0 0 0 0 
0 0 0 0 
Stampa copia t in b
83 77 93 
86 15 35 
*** Error in `/home/ibanez89/UniBa/Workspace/[ASD 2015]/[ASD]Esercitazione_lab3/Debug/[ASD]Esercitazione_lab3': free(): invalid pointer: 0x0000000001f63e10 ***
======= Backtrace: =========
/usr/lib/libc.so.6(+0x72055)[0x7fb1eb9c6055]
/usr/lib/libc.so.6(+0x779a6)[0x7fb1eb9cb9a6]
/usr/lib/libc.so.6(+0x7818e)[0x7fb1eb9cc18e]
/home/ibanez89/UniBa/Workspace/[ASD 2015]/[ASD]Esercitazione_lab3/Debug/[ASD]Esercitazione_lab3[0x400e82]
/home/ibanez89/UniBa/Workspace/[ASD 2015]/[ASD]Esercitazione_lab3/Debug/[ASD]Esercitazione_lab3[0x400c3e]
/usr/lib/libc.so.6(__libc_start_main+0xf0)[0x7fb1eb974610]
/home/ibanez89/UniBa/Workspace/[ASD 2015]/[ASD]Esercitazione_lab3/Debug/[ASD]Esercitazione_lab3[0x400a09]
======= Memory map: ========
00400000-00402000 r-xp 00000000 08:03 14945604                           /home/ibanez89/UniBa/Workspace/[ASD 2015]/[ASD]Esercitazione_lab3/Debug/[ASD]Esercitazione_lab3
00601000-00602000 rw-p 00001000 08:03 14945604                           /home/ibanez89/UniBa/Workspace/[ASD 2015]/[ASD]Esercitazione_lab3/Debug/[ASD]Esercitazione_lab3
01f52000-01f84000 rw-p 00000000 00:00 0                                  [heap]
7fb1e4000000-7fb1e4021000 rw-p 00000000 00:00 0 
7fb1e4021000-7fb1e8000000 ---p 00000000 00:00 0 
7fb1eb954000-7fb1ebaef000 r-xp 00000000 08:02 1051822                    /usr/lib/libc-2.22.so
7fb1ebaef000-7fb1ebcee000 ---p 0019b000 08:02 1051822                    /usr/lib/libc-2.22.so
7fb1ebcee000-7fb1ebcf2000 r--p 0019a000 08:02 1051822                    /usr/lib/libc-2.22.so
7fb1ebcf2000-7fb1ebcf4000 rw-p 0019e000 08:02 1051822                    /usr/lib/libc-2.22.so
7fb1ebcf4000-7fb1ebcf8000 rw-p 00000000 00:00 0 
7fb1ebcf8000-7fb1ebd0e000 r-xp 00000000 08:02 1052090                    /usr/lib/libgcc_s.so.1
7fb1ebd0e000-7fb1ebf0d000 ---p 00016000 08:02 1052090                    /usr/lib/libgcc_s.so.1
7fb1ebf0d000-7fb1ebf0e000 rw-p 00015000 08:02 1052090                    /usr/lib/libgcc_s.so.1
7fb1ebf0e000-7fb1ec00b000 r-xp 00000000 08:02 1051873                    /usr/lib/libm-2.22.so
7fb1ec00b000-7fb1ec20a000 ---p 000fd000 08:02 1051873                    /usr/lib/libm-2.22.so
7fb1ec20a000-7fb1ec20b000 r--p 000fc000 08:02 1051873                    /usr/lib/libm-2.22.so
7fb1ec20b000-7fb1ec20c000 rw-p 000fd000 08:02 1051873                    /usr/lib/libm-2.22.so
7fb1ec20c000-7fb1ec37e000 r-xp 00000000 08:02 1061398                    /usr/lib/libstdc++.so.6.0.21
7fb1ec37e000-7fb1ec57e000 ---p 00172000 08:02 1061398                    /usr/lib/libstdc++.so.6.0.21
7fb1ec57e000-7fb1ec588000 r--p 00172000 08:02 1061398                    /usr/lib/libstdc++.so.6.0.21
7fb1ec588000-7fb1ec58a000 rw-p 0017c000 08:02 1061398                    /usr/lib/libstdc++.so.6.0.21
7fb1ec58a000-7fb1ec58e000 rw-p 00000000 00:00 0 
7fb1ec58e000-7fb1ec5b0000 r-xp 00000000 08:02 1051821                    /usr/lib/ld-2.22.so
7fb1ec763000-7fb1ec769000 rw-p 00000000 00:00 0 
7fb1ec7ad000-7fb1ec7af000 rw-p 00000000 00:00 0 
7fb1ec7af000-7fb1ec7b0000 r--p 00021000 08:02 1051821                    /usr/lib/ld-2.22.so
7fb1ec7b0000-7fb1ec7b1000 rw-p 00022000 08:02 1051821                    /usr/lib/ld-2.22.so
7fb1ec7b1000-7fb1ec7b2000 rw-p 00000000 00:00 0 
7ffdcc751000-7ffdcc772000 rw-p 00000000 00:00 0                          [stack]
7ffdcc794000-7ffdcc796000 r--p 00000000 00:00 0                          [vvar]
7ffdcc796000-7ffdcc798000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
  • Once you get your destructor fixed, and provide the user-defined copy constructor (which is missing), the assignment operator can be just 5 lines: `{matrice temp(m); std::swap(temp.righe, righe); std::swap(temp.colonne, colone); std::swap(temp.elementi, elementi); return *this;}` – PaulMcKenzie Dec 20 '15 at 20:03
  • `this->~matrice();` What are you trying to do here? As soon as you do this, then all bets are off as to how your program will behave. Never call the destructor explicitly, unless you are using `placement-new`, which you're not using. Please work on providing a *correct* 1) copy constructor, 2) destructor, and 3) assignment operator. Without all 3 of these functions working properly, the rest of the code will not behave correctly. – PaulMcKenzie Dec 20 '15 at 20:20
  • Too much work :) All you want to do is remove the old data from the existing object and copy the new data from the passed-in object. See my answer. – PaulMcKenzie Dec 20 '15 at 20:49
  • @PaulMcKenzie sorry for external link but i dont know how to formatting my text here (my first time here) http://pastebin.com/yME3ChGh Sorry for my garbage code, my experience in c++ is very low, this is my first project – Luciano Faretra Dec 20 '15 at 20:53
  • @PaulMcKenzie i will try now, your code look amazing :) – Luciano Faretra Dec 20 '15 at 20:54

2 Answers2

2

The issue has to do with your assignment operator. You are explicitly calling the destructor on this line:

 this->~matrice();

You should not call the destructor explicitly, unlesss you're using placement-new. What is happening is that when the "normal" destructor is called when the object goes out of scope, you are deallocating the same pointer value twice, thus causing undefined behavior to occur.

If we combine the answers given to you for the destructor fixes, then your assignment operator can be written easily. However, your code is missing a user defined copy constructor. Following the Rule of 3, if you have a user-defined destructor, then a user-defined copy constructor and assignment operator should exist.

So let's define a copy constructor for your class:

First, the class is missing this:

matrice(const matrice& rhs);

We want to copy the rhs to the new object. The implementation can look like this:

matrice::matrice(const matrice& rhs) : colonne(rhs.colonne), 
                                       righe(rhs.righe), 
                                       elementi(new tipoelem*[rhs.righe])
{
    for (auto i = 0; i != righe; i++)
        this->elementi[i] = new tipoelem[colonne];

    for (auto i = 0; i != righe; i++)
        for (auto j = 0; j != colonne; j++)
            this->elementi[i][j] = rhs.elementi[i][j];
}

So we copy all the items from rhs, create the matrix, and then copy the matrix values from rhs to this.

Once we have this, then the assignment operator can be written easily using the Copy / Swap Idiom:

matrice& matrice::operator=(const matrice &m) 
{
    matrice temp(m);
    std::swap(temp.colonne, colonne);
    std::swap(temp.righe, righe);
    std::swap(temp.elementi, elementi);
    return *this;
}

This works by creating a temporary copy of rhs (the copy constructor did this job), and then swap out the internals of the existing object with the temporary copy. Since the temporary object now has the old data swapped into it, when it dies (at the end of the function), that old data gets deleted with it. That in a nutshell is what copy / swap does and why you must have a working copy constructor and destructor to use it.

Once we have this, and combine the fixes from the other answer for the destructor, we see that the code now does not crash:

Full Example

Please Note: I used a constant 100.0 instead of rand(), as your code does not compile due to random returning void.

Community
  • 1
  • 1
PaulMcKenzie
  • 31,493
  • 4
  • 19
  • 38
1

You mixed colone with righe

matrice::~matrice(void){
    for(auto j=0;j<this->righe;++j){
                      // ^^^^^^
        delete[] this->elementi[j];
    }
    delete[] this->elementi;
}

.. and there too ...

matrice matrice::matriceProdotto(matrice& M){
    matrice prodotto(righe, colonne);

    for(auto i=0; i<righe;i++)
        for(auto j=0; j<colone ;j++)
                     // ^^^^^^^
            prodotto.scriviMatrice(i,j,(matrice::leggiMatrice(i,j)*M.leggiMatrice(i,j)));
    return prodotto;
}
Rabbid76
  • 142,694
  • 23
  • 71
  • 112