2017-03-08 3 views
2

Je suis en train me C++, et je suis vraiment obtenir un peu confus.
j'ai vraiment eu des problèmes avec le constructeur compréhension/Copier Constructeur/Déplacer Constructeur/Destructeur et leur mise en place correctement.C++ - Constructeur, constructeur de copie, Déplacer Constructor, Destructeur

Je veux mettre en œuvre une copie ainsi qu'un constructeur de déplacement et un destructor dans le class Container. J'ai d'abord essayé le constructeur de la copie et j'ai pensé que je l'avais bien fait. Malheureusement, le destructeur laisse le programme se bloquer à la fin pour une raison quelconque en essayant de faire . Je pense que j'ai eu quelque chose de mal avant que le champ data ne soit plus là et que je l'ai copié mal ou quoi que ce soit. J'espère que vous pouvez m'aider et comprendre mes erreurs. J'ai aussi essayé un constructeur de mouvement (en dessous), qui ne fonctionne absolument pas.

Merci pour l'aide les gars. En espérant pouvoir donner quelque chose plus tard :)

classe Container:

#include <iostream> 
#include <memory> 

class Container 
{ 
public: 
Container() 
{ 
    length = 0; 
    data = nullptr; 
} 

Container(int lengthin):Container() 
{ 
    length = lengthin; 
    data = new double[lengthin]; 
    //data[lengthin]; 
    //double data[lengthin] = {0}; 
} 

Container(std::initializer_list<double> listin) 
     :Container((int)listin.size()) 
{ 
    std::uninitialized_copy (listin.begin(), listin.end(), data); 
} 

//copy constructor - working? 
Container(const Container& other):Container(other.length) 
{ 
    //data = other.data; 
    //length = other.length; 
    for (auto i=0; i<other.length; i++) 
    { 
     data[i] = other.data[i]; 
    } 
} 


//~Container(){length = 0;} 
~Container() 
{ 
    delete[] data; 
    length = 0; 
} 


Container operator+(Container cin) 
{ 
    Container cout(cin.length); 
    cout.length = cin.length; 
    for (auto i=0; i<cin.length; i++) 
    { 
     cout.data[i] = cin.data[i] + data[i]; 
    } 
    return cout; 
} 

Container operator-(Container cin) 
{ 
    Container cout(cin.length); 
    cout.length = cin.length; 
    for (auto i=0; i<cin.length; i++) 
    { 
     cout.data[i] = data[i]-cin.data[i]; 
    } 
    return cout; 
} 


void print(const std::string &info) const 
{ 
    // print the address of this instance, the attributes `length` and 
    // `data` and the `info` string 
    std::cout << " " << this << " " << length << " " << data << " " 
       << info << std::endl; 
} 

private: 
    int length; 
    double *data; 
}; 

Le programme principal:

int main() 
{ 
Container a({ 1, 2, 3 }); 
std::cout << " a has address " << &a << std::endl; 

Container b = { 4, 5, 6 }; 
std::cout << " b has address " << &b << std::endl; 

Container c(a); 
std::cout << " c has address " << &c << std::endl; 

Container d = a + b; 
std::cout << " d has address " << &d << std::endl; 

Container e; 
std::cout << " e has address " << &e << std::endl; 

e = a + b; 

//Container f(std::move(a + b)); 
//std::cout << " f has address " << &f << std::endl; 

return 0;} 

Et le constructeur move essayé:

Container(const Container&& other):length(other.length), data(other.data) 

Répondre

1

Votre constructeur de mouvement ne déplacer quoi que ce soit, je t copie juste la longueur et le pointeur (ce qui signifie qu'il fonctionne exactement comme le constructeur de copie par défaut). Cela signifie que vous aurez deux objets Container dont le membre data pointera vers la même mémoire.

Lorsque l'un des objets que la mémoire est supprimé, conduisant à comportement non défini lorsque le deuxième objet tente de supprimer la même mémoire.

Une manière simple de résoudre ce problème consiste à définir les autres objets length sur zéro et data sur nullptr. Ou par défaut initialiser l'objet en cours, puis échanger les deux objets.

1

Comme l'a expliqué Un mec programmeur, ce constructeur de mouvement ferait:

Container(Container&& other):length(other.length), data(other.data) 
{ 
other.length = 0; 
other.data = nullptr; 
} 

other.data = nullptr; donc quand destructor d'autre est appelée, delete []data aura aucun effet (avec votre code actuel est Invalide le tableau de données détenues par la conteneur que vous venez d'emménager à. other.length = 0; parce que depuis l'autre n'a pas de tableau de données, il doit également avoir aucune longueur. Notez que je ne posté le code, car apparemment vous avez posté une mauvaise réponse vous (il semble que la première réponse n'a pas été assez clair quant à ce que le code devrait être.) parce que c'est un constructeur ne vous ont pas à vous soucier de this->data noter que avec un opérateur asignement move vous devez d'abord delete this->data[] afin d'éviter la fuite de mémoire.

La fonction de copie asignement pourrait être en tant que tel:

Container& operator=(const Container &other) 
{ 
    if (this->length) 
    delete[] this->data; 
    this -> length = other.length; 
    data = new double [this->length]; 
    for (auto i = 0; i<other.length; i++) 
    { 
     this->data[i] = other.data[i]; 
    } 
    return *this; 
} 
1

Votre constructeur de copie est très bien (mais peut être simplifiée en utilisant un algorithme de copie au lieu d'une boucle manuelle).

Il manque à votre classe un opérateur d'affectation de copie, un constructeur de déplacement et un opérateur d'affectation de mouvement.

Et vos operator+ et devraient prendre leurs entrées par référence plutôt que par valeur, et être eux-mêmes déclarés comme const. Ils ne tiennent pas compte non plus du fait que l'entrée Container peut avoir un length différent de celui du Container utilisé.

Essayez quelque chose comme ceci:

#include <iostream> 
#include <algorithm> 

class Container 
{ 
public: 
    Container() : length(0), data(nullptr) 
    { 
    } 

    Container(int len) : Container() 
    { 
     length = len; 
     data = new double[len]; 
    } 

    Container(std::initializer_list<double> src) : Container((int)src.size()) 
    { 
     std::uninitialized_copy(src.begin(), src.end(), data); 
    } 

    Container(const Container &src) : Container(src.length) 
    { 
     std::uninitialized_copy(src.data, src.data + src.length, data); 
    } 

    Container(Container &&src) : Container() 
    { 
     src.swap(*this); 
    } 

    ~Container() 
    { 
     delete[] data; 
     length = 0; 
    } 

    void swap(Container &other) noexcept 
    { 
     std::swap(data, other.data); 
     std::swap(length, other.length); 
    } 

    Container& operator=(const Container &rhs) 
    { 
     if (length < rhs.length) 
     { 
      Container tmp(rhs); 
      swap(tmp); 
     } 
     else 
     { 
      length = rhs.length; 
      std::uninitialized_copy(rhs.data, rhs.data + rhs.length, data); 
     } 
     return *this;   
    } 

    Container& operator=(Container&& rhs) 
    { 
     rhs.swap(*this); 
     return *this;   
    } 

    Container operator+(const Container &rhs) const 
    { 
     int len = std::max(length, rhs.length); 
     Container out(len); 
     for (auto i = 0; i < len; ++i) 
     { 
      if ((i < length) && (i < rhs.length)) 
       out.data[i] = data[i] + rhs.data[i]; 
      else 
       out[i] = (i < length) ? data[i] : rhs.data[i]; 
     } 
     return out; 
    } 

    Container operator-(const Container &rhs) const 
    { 
     int len = std::max(length, rhs.length); 
     Container out(len); 
     for (auto i = 0; i < len; ++i) 
     { 
      if ((i < length) && (i < rhs.length)) 
       out.data[i] = data[i] - rhs.data[i]; 
      else 
       out[i] = (i < length) ? data[i] : rhs.data[i]; 
     } 
     return out; 
    } 

    void print(const std::string &info) const 
    { 
     // print the address of this instance, the attributes `length` and 
     // `data` and the `info` string 
     std::cout << " " << this << " " << length << " " << data << " " << info << std::endl; 
    } 

private: 
    int length; 
    double *data; 
}; 

namespace std 
{ 
    void swap(Container &lhs, Container &rhs) 
    { 
     lhs.swap(rhs); 
    } 
} 

Et vous pouvez simplifier grandement les choses en utilisant std::vector au lieu d'un tableau manuel:

#include <iostream> 
#include <algorithm> 
#include <vector> 

class Container 
{ 
public: 
    Container(size_t len = 0) : data(len) 
    { 
    } 

    Container(std::initializer_list<double> src) : data(src) 
    { 
    } 

    void swap(Container &other) noexcept 
    { 
     std::swap(data, other); 
    } 

    Container operator+(const Container &rhs) const 
    { 
     size_t thislen = data.size(); 
     size_t thatlen = rhs.data.size(); 
     size_t len = std::max(thislen, thatlen); 
     Container out(len); 
     for (auto i = 0; i < len; ++i) 
     { 
      if ((i < thislen) && (i < thatlen)) 
       out.data[i] = data[i] + rhs.data[i]; 
      else 
       out[i] = (i < thislen) ? data[i] : rhs.data[i]; 
     } 
     return out; 
    } 

    Container operator-(const Container &rhs) const 
    { 
     size_t thislen = data.size(); 
     size_t thatlen = rhs.data.size(); 
     size_t len = std::max(thislen, thatlen); 
     Container out(len); 
     for (auto i = 0; i < len; ++i) 
     { 
      if ((i < thislen) && (i < thatlen)) 
       out.data[i] = data[i] - rhs.data[i]; 
      else 
       out[i] = (i < thislen) ? data[i] : rhs.data[i]; 
     } 
     return out; 
    } 

    void print(const std::string &info) const 
    { 
     // print the address of this instance, the attributes `length` and 
     // `data` and the `info` string 
     std::cout << " " << this << " " << data.size() << " " << data.data() << " " << info << std::endl; 
    } 

private: 
    std::vector<double> data; 
}; 

namespace std 
{ 
    void swap(Container &lhs, Container &rhs) 
    { 
     lhs.swap(rhs); 
    } 
}