Question

C’est un peu une question pour les débutants, mais je n’ai pas utilisé le C ++ depuis longtemps, alors allez-y ...

J'ai une classe qui contient un tableau alloué dynamiquement, disons

class A
{
    int* myArray;
    A()
    {
        myArray = 0;
    }
    A(int size)
    {
        myArray = new int[size];
    }
    ~A()
    {
        // Note that as per MikeB's helpful style critique, no need to check against 0.
        delete [] myArray;
    }
}

Mais maintenant, je veux créer un tableau alloué dynamiquement de ces classes. Voici mon code actuel:

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i] = A(3);
}

Mais cela explose terriblement. Parce que le nouvel A objet créé (avec l'appel A(3)) est détruit à la fin de l'itération de la boucle for, cela signifie que le myArray interne de cette instance delete [] reçoit vector<int> - ed.

Donc, je pense que ma syntaxe doit être terriblement fausse? J'imagine que certains correctifs semblent excessifs, ce que j'espère éviter:

  • Création d'un constructeur de copie pour vector<A>.
  • Utiliser arrayOfAs et A* pour ne pas m'inquiéter de tout cela.
  • Au lieu d'avoir vector un tableau d'objets <=>, choisissez un tableau de <=> pointeurs.

Je pense que ce n’est qu’un débutant pour lequel il existe une syntaxe qui fonctionne lorsque vous essayez d’allouer de manière dynamique un tableau de choses qui ont une allocation dynamique interne.

(En outre, les critiques de style ont été appréciées, car cela fait longtemps que je n’utilise pas C ++.)

Mettre à jour pour les futurs téléspectateurs : toutes les réponses ci-dessous sont vraiment utiles. Martin's est accepté en raison de l'exemple de code et de l'utile & Quot; règle de 4, & Quot; mais je suggère vraiment de les lire tous. Certaines sont de bonnes déclarations succinctes de ce qui ne va pas, et certaines indiquent correctement comment et pourquoi <=> sont une bonne solution.

Était-ce utile?

La solution

Pour construire des conteneurs, vous souhaitez évidemment utiliser l'un des conteneurs standard (tel qu'un std :: vector). Mais c’est un exemple parfait de ce que vous devez prendre en compte lorsque votre objet contient des pointeurs RAW.

Si votre objet a un pointeur RAW, vous devez vous rappeler la règle de 3 (maintenant la règle de 5 en C ++ 11).

  • constructeur
  • Destructor
  • Constructeur de copie
  • Opérateur d'affectation
  • Constructeur Move (C ++ 11)
  • Déplacer une affectation (C ++ 11)

En effet, s'il n'est pas défini, le compilateur générera sa propre version de ces méthodes (voir ci-dessous). Les versions générées par le compilateur ne sont pas toujours utiles lorsque vous utilisez des pointeurs RAW.

Le constructeur de copie est difficile à corriger (ce n'est pas trivial si vous voulez fournir la garantie d'exception forte). L'opérateur Assignment peut être défini en termes de constructeur de copie, car vous pouvez utiliser l'idiome copy et swap en interne.

Voir ci-dessous pour plus de détails sur le minimum absolu pour une classe contenant un pointeur sur un tableau d'entiers.

Sachant qu'il est non trivial de le corriger, vous devriez envisager d'utiliser std :: vector plutôt qu'un pointeur sur un tableau d'entiers. Le vecteur est facile à utiliser (et à développer) et couvre tous les problèmes associés aux exceptions. Comparez la classe suivante à la définition de A ci-dessous.

class A
{ 
    std::vector<int>   mArray;
    public:
        A(){}
        A(size_t s) :mArray(s)  {}
};

En regardant votre problème:

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    // As you surmised the problem is on this line.
    arrayOfAs[i] = A(3);

    // What is happening:
    // 1) A(3) Build your A object (fine)
    // 2) A::operator=(A const&) is called to assign the value
    //    onto the result of the array access. Because you did
    //    not define this operator the compiler generated one is
    //    used.
}

L'opérateur d'affectation généré par le compilateur convient à presque toutes les situations, mais lorsque des pointeurs RAW sont en jeu, vous devez faire attention. Dans votre cas, cela pose un problème en raison du problème de copie superficielle . Vous vous êtes retrouvé avec deux objets contenant des pointeurs sur le même fragment de mémoire. Lorsque le A (3) sort de la portée à la fin de la boucle, il appelle delete [] sur son pointeur. Ainsi, l’autre objet (dans le tableau) contient maintenant un pointeur sur la mémoire qui a été renvoyé au système.

Le constructeur de copie généré par le compilateur ; copie chaque variable de membre en utilisant le constructeur de la copie de membres. Pour les pointeurs, cela signifie simplement que la valeur du pointeur est copiée de l'objet source vers l'objet de destination (d'où une copie superficielle).

Opérateur d’affectation généré par le compilateur ; copie chaque variable de membre en utilisant cet opérateur d'affectation de membres. Pour les pointeurs, cela signifie simplement que la valeur du pointeur est copiée de l'objet source vers l'objet de destination (d'où une copie superficielle).

Donc, le minimum pour une classe contenant un pointeur:

class A
{
    size_t     mSize;
    int*       mArray;
    public:
         // Simple constructor/destructor are obvious.
         A(size_t s = 0) {mSize=s;mArray = new int[mSize];}
        ~A()             {delete [] mArray;}

         // Copy constructor needs more work
         A(A const& copy)
         {
             mSize  = copy.mSize;
             mArray = new int[copy.mSize];

             // Don't need to worry about copying integers.
             // But if the object has a copy constructor then
             // it would also need to worry about throws from the copy constructor.
             std::copy(&copy.mArray[0],&copy.mArray[c.mSize],mArray);

         }

         // Define assignment operator in terms of the copy constructor
         // Modified: There is a slight twist to the copy swap idiom, that you can
         //           Remove the manual copy made by passing the rhs by value thus
         //           providing an implicit copy generated by the compiler.
         A& operator=(A rhs) // Pass by value (thus generating a copy)
         {
             rhs.swap(*this); // Now swap data with the copy.
                              // The rhs parameter will delete the array when it
                              // goes out of scope at the end of the function
             return *this;
         }
         void swap(A& s) noexcept
         {
             using std::swap;
             swap(this.mArray,s.mArray);
             swap(this.mSize ,s.mSize);
         }

         // C++11
         A(A&& src) noexcept
             : mSize(0)
             , mArray(NULL)
         {
             src.swap(*this);
         }
         A& operator=(A&& src) noexcept
         {
             src.swap(*this);     // You are moving the state of the src object
                                  // into this one. The state of the src object
                                  // after the move must be valid but indeterminate.
                                  //
                                  // The easiest way to do this is to swap the states
                                  // of the two objects.
                                  //
                                  // Note: Doing any operation on src after a move 
                                  // is risky (apart from destroy) until you put it 
                                  // into a specific state. Your object should have
                                  // appropriate methods for this.
                                  // 
                                  // Example: Assignment (operator = should work).
                                  //          std::vector() has clear() which sets
                                  //          a specific state without needing to
                                  //          know the current state.
             return *this;
         }   
 }

Autres conseils

Je recommanderais d'utiliser std :: vector: quelque chose comme

typedef std::vector<int> A;
typedef std::vector<A> AS;

Il n'y a rien de mal avec le léger surdimensionnement de STL et vous pourrez passer plus de temps à mettre en œuvre les fonctionnalités spécifiques de votre application au lieu de réinventer le vélo.

Le constructeur de votre objet A affecte dynamiquement un autre objet et stocke un pointeur sur cet objet alloué de manière dynamique dans un pointeur brut.

Pour ce scénario, vous devez définir votre propre constructeur de copie, votre opérateur d'assignation et votre destructeur. Ceux générés par le compilateur ne fonctionneront pas correctement. (Ceci est un corollaire à la & "Loi des Trois Grands &"; Une classe avec un destructeur, un opérateur d’affectation ou un constructeur de copie nécessite généralement les 3).

Vous avez défini votre propre destructeur (et vous avez mentionné la création d'un constructeur de copie), mais vous devez définir les deux autres des deux grands trois.

Une alternative consiste à stocker le pointeur sur votre int[] alloué dynamiquement dans un autre objet qui s’occupera de ces tâches pour vous. Quelque chose comme un vector<int> (comme vous l'avez mentionné) ou un boost::shared_array<>.

En résumé, pour tirer pleinement parti de la norme RAII, vous devez éviter autant que possible de traiter les indicateurs bruts.

Et puisque vous avez demandé d'autres critiques de style, une remarque mineure est que, lorsque vous supprimez des pointeurs bruts, vous n'avez pas besoin de vérifier 0 avant d'appeler. delete - <=> traite ce cas en ne faisant rien, vous ne ' pas besoin de vous encombrer de code avec les contrôles.

  1. Utilisez un conteneur array ou common pour les objets uniquement s'ils ont des constructeurs par défaut et des constructeurs de copie.

  2. Stockez les pointeurs autrement (ou les pointeurs intelligents, mais peut rencontrer certains problèmes dans ce cas).

PS: définissez toujours vos propres constructeurs par défaut et leurs constructeurs de copie, sinon les fichiers générés automatiquement seront utilisés

Vous avez besoin d'un opérateur d'affectation pour que:

arrayOfAs[i] = A(3);

fonctionne comme il se doit.

Pourquoi ne pas avoir une méthode setSize.

A* arrayOfAs = new A[5];
for (int i = 0; i < 5; ++i)
{
    arrayOfAs[i].SetSize(3);
}

J'aime bien la " copie " mais dans ce cas, le constructeur par défaut ne fait rien. SetSize pourrait copier les données hors du m_array d'origine (s'il existe). Vous devez stocker la taille du tableau dans la classe pour le faire.
OU
SetSize pourrait supprimer le m_array d'origine.

void SetSize(unsigned int p_newSize)
{
    //I don't care if it's null because delete is smart enough to deal with that.
    delete myArray;
    myArray = new int[p_newSize];
    ASSERT(myArray);
}

En utilisant la fonctionnalité de positionnement de l'opérateur new, vous pouvez créer l'objet à la place et éviter de le copier:

  

placement (3): opérateur void * new (std :: size_t size, void * ptr) noexcept;

     

Renvoie simplement ptr (aucun stockage n'est alloué).   Notez cependant que si la fonction est appelée par une nouvelle expression, l’initialisation appropriée sera effectuée (pour les objets de classe, cela inclut l’appel de son constructeur par défaut).

Je suggère ce qui suit:

A* arrayOfAs = new A[5]; //Allocate a block of memory for 5 objects
for (int i = 0; i < 5; ++i)
{
    //Do not allocate memory,
    //initialize an object in memory address provided by the pointer
    new (&arrayOfAs[i]) A(3);
}
Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top