Dfyjkt

I have two classes. For illustrative purposes, I'm using the idea of a menu and menu items

class Menu 
 public:
 ...
 RemoveItem(Item* item) 
 // Remove appropriate item from menu_items vector
 ;

 private:
 std::vector<std::shared_ptr<Item>> menu_items;


class Item 
 public:
 Item(Menu* owner) : menuowner 

 ~Item() RemoveThisMenuItem() 

 void RemoveThisMenuItem() 
 for (const auto& ingredient : ingredients) 
 ingredient.SetNecessary(false);
 
 menu.RemoveItem(this);
 
 ...

 private:
 Menu* menu;
 std::vector<Ingredients*> ingredients;
 ...

Essentially, I have a class Item that is owned by a class Menu (and possibly referenced elsewhere with shared_ptrs). Now, I want to remove an Item. When Item is deleted, it needs to perform some other functions first (like marking all of the ingredients unnecessary) and then remove it from the Menu.

The way things are implemented now, there's a strange loop when we want to remove the item, since calling Item::RemoveThisMenuItem() calls a function in Menu that removes the smart pointer to the Item, which calls the destructor ~Item(), which then calls Item::RemoveThisMenuItem(). What's the best way to get around this?

Make another copy of the shared_ptr which owns the item, and will destroy it when it goes out of scope.

i.e. instead of doing:

menu_items[n]->RemoveThisMenuItem();

do:

 auto owner = menu_items[n];
 owner->RemoveThisMenuItem();

Now when RemoveThisMenuItem() erases the element from the vector it isn't the last object that shares ownership of the Item, and so it won't be destroyed until owner goes out of scope.

I'd change method void RemoveItem(Item* item) to std::shared_ptr<Item> RemoveItem(Item* item) and implement it in a way that it moves/copies the respective item-shared_ptr out of the vector and returns it; if the item has already been removed, return an empty shared ptr to avoid endless recursion. This way, you prevent RemoveItem to ocassionally delete the Item-object (in case the last shared_ptr of it was destroyed) and delegate the topic to the caller of RemoveItem. The caller can then decide if to keep such a shared_pointer (and thereby the corresponding object) alive or not.

For example:

void RemoveThisMenuItem() 
 for (const auto& ingredient : ingredients) 
 ingredient.SetNecessary(false);
 
 auto ptr = menu.RemoveItem(this);
 // at that point, "this" will have not been destroyed;
 // when the method is left, however, ptr will be destroyed and the destructor will be called;