¿Llamada al destructor en relación de clases Composición? (C++)

Question

Un saludo.

Estoy realizando una práctica de POO en C++. La temática (no entiendo por qué) es de algo de StarWars. La cosa es que una nave (StarFighter) está formada por una serie de piezas (un atributo en un array de punteros a piezas), como queda detallado en el fichero de cabeceras:

#ifndef STARFIGHTER_H
#define STARFIGHTER_H

#include <string>
#include <stdexcept>
#include "Pieza.h"

/**
 * @brief
 */
class StarFighter
{
   private:
      static const int MAX_PIEZAS = 50;
      static int _numStarFighters; ///< Número de objetos de esta clase instanciados
      int _idSF=0; ///< Identificador único de la nave
      std::string _marca; ///< Marca de la nave (parece que las VW contaminan más)
      std::string _modelo; ///< Modelo de la nava
      int _numPlazas = 1; ///< Número de plazas de la nave
      int _numPiezas = 1; ///< Numero de piezas de la nave
      Pieza *_piezas[MAX_PIEZAS]; ///<Array de piezas de la nave
   public:
      StarFighter ();
      StarFighter ( std::string marca, std::string modelo, int numpiezas );
      StarFighter ( const StarFighter& orig );
      virtual ~StarFighter ( );
      StarFighter& setNumPlazas ( int numPlazas );
      int getNumPlazas ( ) const;
      StarFighter& setModelo ( std::string modelo );
      std::string getModelo ( ) const;
      StarFighter& setMarca ( std::string marca );
      std::string getMarca ( ) const;
      int getIdSF ( ) const;
      std::string toCSV () const;
      StarFighter& operator= ( const StarFighter& otro );
      void fromCSV ( std::string& datos );
      void addPieza(std::string nombre, float peso, std::string descripcion);
      void quitarPieza(int posicion);
};

#endif /* STARFIGHTER_H */

El problema viene en la destrucción de la nave. Al ser composición, es la nave la que se debe encargar de destruir los objetos de tipo pieza, pero no sé como tengo que implementar el destructor para que eso ocurra. Yo lo tengo ahora mismo así:

StarFighter::~StarFighter ( )
{
    for (int i = 0; i < MAX_PIEZAS; i++){
        this->_piezas[i] = nullptr;
    }
}

Pero eso no llama nunca al destructor de la clase Pieza (lo he comprobado con el debugger), por lo que entiendo que se producirá goteo de memoria. He probado a cambiar el constructor y ponerlo como sigue:

StarFighter::~StarFighter ( )
{
    for (int i = 0; i < MAX_PIEZAS; i++){
        delete [] this->_piezas;
    }
}

Lo cual me lleva a mis ya familiares "Segmentation Fault" (no entiendo por qué tampoco, pero bueno).

Antes de terminar, por si ha quedado en el aire, aclaro que el constructor crea piezas hasta el número que se le indique (por defecto 1):

StarFighter::StarFighter ( string marca, string modelo, int numpiezas ):
                           _marca (marca), _modelo(modelo), _numPiezas(numpiezas)
{
   _numStarFighters++;
   _idSF = _numStarFighters;
   for (int i = 0; i < MAX_PIEZAS; i++){
       _piezas[i] = nullptr; //Inicializamos todo el vector a nullptr
   }
   for (int i = 0; i < _numPiezas; i++){
       _piezas[i] = new Pieza;
   }
}

Agradezco cualquier sugerencia. Esta asignatura tiene tarea, y estar sin clases encima no ayuda.

Muchas gracias.

Answer 1

Veamos tus intentos:

StarFighter::~StarFighter ( )
{
    for (int i = 0; i < MAX_PIEZAS; i++){
        this->_piezas[i] = nullptr;
    }
}

Con eso, tal y como dices, no liberas memoria. Tus sospechas son ciertas. Asignas un nuevo valor a cada puntero, pero la memoria a la que apuntaba nunca se libera.

StarFighter::~StarFighter ( )
{
    for (int i = 0; i < MAX_PIEZAS; i++){
        delete [] this->_piezas;
    }
}

aclaro que el constructor crea piezas hasta el número que se le indique (por defecto 1):

Pues ya lo tienes. Ese código intenta liberar todo el contenido de la formación, pero esto es incorrecto ya que no todos los elementos son punteros válidos. El intentar liberar un bloque de memoria que no ha sido previamente reservado es un comportamiento indefinido. Y una posible consecuencia de eso es lo que ya has observado: Segmentation Fault.

La forma correcta sería ... pues simplemente, hacer lo mismo que en el constructor, pero, en lugar de llamar a new, pues llamando a delete :-)

StarFighter::~StarFighter ( )
{
    for (int i = 0; i < _numPiezas; i++){
        delete _piezas[i];
    }
}

Answer 2

es la nave la que se debe encargar de destruir los objetos de tipo pieza, pero no sé como tengo que implementar el destructor para que eso ocurra.

Eso depende de cómo se construyan, viendo que almacenas una colección de punteros:

Pieza *_piezas[MAX_PIEZAS];
//    ^ <--- Punteros

Que rellenas con new:

_piezas[i] = new Pieza;

Deberás borrar cada una con delete:

StarFighter::~StarFighter ( )
{
    for (pieza : _piezas)
        delete pieza;
}

Hasta aquí la duda, lo mejor que puede ser respondida con los pocos datos que das. Ahora un repaso a el resto de cosas incorrectas o mejorables de tu código:

1. No uses subguiones para nombrar variables, según el estándar de C++ §5.10 (traducción y resaltado míos):

   5.10 Identificadores.

   3. Adicionalmente, algunos identificadores están reservados para ser usados por las implementaciones de C++ y no deben usarse en cualquier otro caso; no se requiere diagnóstico.
      1. Cada identificador que contenga un doble subguión __ o empiece por un subguión seguido de una letra mayúscula se reserva para la implementación en cualquier uso.
      2. Cada identificador que empiece con un subguión está reservado para la implementación para ser usado en el espacio de nombres global.

En esta pregunta se habla del tema. Tus variables miembro no cumplen ninguno de los dos puntos, pero es mejor evitar esos nombres por si entran en conflicto con otros nombres.

2. El uso de punteros en crudo está desaconsejado en C++ moderno, también se considera poco práctico el uso de formaciones. Deberías cambiar la formación por un std::array formado por punteros inteligentes std::unique_ptr:

   class StarFighter
   {
       using pieza_t = std::unique_ptr<Pieza>;
       static constexpr int MAX_PIEZAS = 50;
       std::array<pieza_t, MAX_PIEZAS> piezas;
   

   He eliminado la etiqueta private al inicio de la clase porque esa es la visibilidad por defecto de los objetos tipo class, también he creado un alias del tipo puntero único a Pieza que facilita su uso dentro del objeto y finalmente he indicado que la constante MAX_PIEZAS es una expresión constante, lo cuál denota su intencionalidad además de permitir al compilador hacer optimizaciones.

   Para añadir objetos Pieza a tu colección, basta con crear un nuevo elemento en la posición correcta:

   piezas[posicion] = std::make_unique<Pieza>();
   

   Y no deberás preocuparte del destructor, porque los punteros inteligentes ya se destruyen automáticamente cuando finaliza su ciclo de vida.