0

I would Like to construct a class that contains itself but I have to avoid an endless loop. For example I started of my class

#include <iostream>
#include <vector>
#include <memory>
using namespace std;

class box {
 protected:
  int id;
  vector<box*> intmega;
  int n = 10;

 public:
  box(box const& autre, int id) : n(autre.n), id(id) {
    for (auto& element : autre.get_intmega()) {
      intmega.push_back(new box(*element, id + 100));
    }
    cout << "A new object has seen light" << endl;
  }
  box(int id) : n(10), id(id) { cout << "Created Box" << endl; }
  void ajoute2(box& autre) { intmega.push_back(new box(autre)); }

  int size_() const { return intmega.size(); }
  int number() const { return n; }

  box* get() { return intmega[0]; }

  vector<box*> get_intmega() const { return intmega; }

  int getId() const { return id; }

  ~box() {
    cout << this << endl;
    for (auto element : this->intmega)
      delete element;
  }
};

void affichel(box const& autre) {
  cout << "Box :" << autre.getId() << endl;
  cout << "Box :" << &autre << endl;
}

void affiche(box& autre) {
  for (auto* element : autre.get_intmega()) {
    affichel(*element);
    affiche(*element);
    cout << endl;
  }
}

int main() {
  box box1(1);
  box box2(2);
  box box3(3);

  box2.ajoute2(box3);
  box1.ajoute2(box2);

  box box4(box1, 4);

  affiche(box1);
  cout << "Box1 Address : " << &box1 << endl;

  affiche(box4);
  cout << "Box4 Address : " << &box4 << endl;

  return 0;
}

Everything works fine but upon calling the destructor disaster. It deletes all objects but it gets into an endless loop of deleting an object that has already been deleted. Any suggestions help?

tadman
  • 208,517
  • 23
  • 234
  • 262
Geordie D
  • 21
  • 5
  • 2
    Tip: `std::shared_ptr` to encapsulate that and handle destruction for you. If you have multiply-referenced pointers, you'll `delete` more than once, hence crash. – tadman May 04 '21 at 18:58
  • 1
    And if you really want to keep the raw pointers you must learn about the [rule of 3 (or 5)](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three#:~:text=The%20rule%20of%20zero,functions%20when%20creating%20your%20class.) – Lukas-T May 04 '21 at 19:00
  • because is my first time encountering such a thing, and I never used shared pointers can you explain a bit more. And I coded based on the fact that each object must be unique. I know I didn't use unique_ptr but it works. Thank you for your immediate responce – Geordie D May 04 '21 at 19:01
  • Subtle side note: You won't run into problems here, but members are always constructed in the order they are defined in in the class definition. This means `id` will always be initialized before `n` even when you `: n(autre.n), id(id)`. This won't hurt here, but if `id`'s initialization depended on the value of `n`, `n` would not have been initialized, possibly leading to much puzzling debugging. – user4581301 May 04 '21 at 19:02
  • You need to define a copy constructor: box(const box & other) – Sergey Aleksandrovich May 04 '21 at 19:03
  • 1
    You shouldn't need a box pointer. You can use the default move and copy constructor. Replace vector intmega width vector intmega, delete your destructor and avoid new Box – Jimmy Loyola May 04 '21 at 19:07
  • The Rule of Three in a nutshell: If you need a destructor, you probably need a Copy constructor and Assignment operator to go with it. There are exceptions to this rule, but this code isn't one of them. `new box(autre)` makes a new `box`, but its `intmega` member points at `box`s that are NOT new. When you copy a `box` you need to copy the `box`s in the `box` as well. – user4581301 May 04 '21 at 19:08

1 Answers1

0

I made those slight modifications and now my program works just fine.

 #include <iostream>
 #include <vector>
 #include <memory>
 using namespace std;

 class box
{
protected:
vector<box*> intmega;
int n = 10;

 public:
box(box const &autre) : n(autre.n)
{
    for (auto &element : autre.get_intmega())
    {
        intmega.push_back(new  box(*element));
    }
    cout << "A new object has seen light" << endl;
    cout<<this<<endl;
}
box()
{
    cout << "Created Box" << endl;
    cout<<this<<endl;
}
void ajoute2(box & autre){
            intmega.push_back(new box(autre));
            }
            
int size_() const
{
    return intmega.size();
}
int number() const
{
    return n;
}

vector<box *> get_intmega() const
{
    return intmega;
}

~box()
{
    cout<<this<<endl;
    for(auto element: this->intmega){
            delete element;
        }
}
 };

    void affichel(box const &autre)
    {
         cout << "Box :" << &autre<< endl;
     }

     void affiche(box &autre)
  {
   for (auto *element : autre.get_intmega())
{
    affichel(*element);
    affiche(*element);
    cout << endl;
}
   }

   int main()
{
 box box1;
 box box2;
 box box3;
 box box4;

box3.ajoute2(box4);
box2.ajoute2(box3);
box1.ajoute2(box2);

box box5(box1);

affiche(box1);
cout << "Box1 Address : " << &box1 << endl ;


affiche(box5);
cout << "Box5 Address : " << &box4 << endl ;

return 0;
 }
cigien
  • 57,834
  • 11
  • 73
  • 112
Geordie D
  • 21
  • 5
  • This looks like it would solve the problem (haven't run it, though), but *heed Jimmy Loyola's comment above.* If you are going to copy `box`s and store copies in a `vector`, you are a lot better off without pointers. The `vector` will do all of the work for you and transparently. Zero effort is required on your part. I believe this is 100% legal as of C++17, but often works as expected in older implementations as well. – user4581301 May 04 '21 at 21:36
  • Please don't ask follow up questions in answers. If this solution works, that's sufficient. You should add some explanation of *what* you changed though. If you do have a new question, then go ahead and post that. – cigien May 05 '21 at 01:51
  • To be honest with you. I used pointers because in reality box is sub-class containing many objects of another class, and I wanted to make something like a stack of my own – Geordie D May 05 '21 at 07:45