Correct way to store objects in another object in C++?

Question

I have 3 object, let's call them Main, Manager & Item. The Manager needs to have an array of Items. These Items are added to the Manager from the Main object.

I'd like to know how should I pass the Items to the Manager in order to make them live even outside the Main() function scope, but at the same time, being able to delete them when the Manager is destroyed.

NOTE Item, inside Manager, have to be a pointer because I need to check for NULL items

So far I have something like this (not actual code for short):

Main
{
    Manager* Man;
    Main()
    {
        Man = new Manager(/**/);   //i use a pointer because i need this object to persist;
        Item* it = new Item(/**/);
        Man->AddItem(it);
    }
    ~Main()
    {
        delete(Man);
    }
}

Manager
{
    Item* ItemArchive[15];
    void AddItem(Item* item)
    {
        ItemArchive[index] = item;
    }
    ~Manager()
    {
        for(int i=0;i<archiveLength;i++)
            delete(ItemArchive[i]);    //Here i get a runtime error,most probably an 
                                       //access violation,can't be more specific
                                       //because Unreal Engine doesn't give me that info
    }
}
Item
{
    //just a basic object
}

So my question is, how can I create the objects in the Main and then being able to use and delete them inside the Manager?

Generally, I would not name functions or other entities Main(), or main(). I would also refrain from naming executables "test" or "ls". — Peter - Reinstate Monica, Dec 12 '14 at 14:33
Generally, cleanup like "delete Items when the Manager is deleted" belongs in the destructor. If you copy pointers around just make sure that the objects they point to don't go out of scope, e.g. by using new or modern unique/shared_ptr templates; and make on the other hand sure they aren't used after the Manager has deleted them (i.e. that the Manager has ownership). The latter can be less than trivial in complex programs, but the modern C++ smart pointer facilities can help. — Peter - Reinstate Monica, Dec 12 '14 at 14:35
"i use a pointer because i need this object to persist" - persist beyond what point? It gets destroyed in the destructor anyway. I don't see why you can't just make the `Manager` a member. — Joseph Mansfield, Dec 12 '14 at 14:36
consider this too http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three — Giorgi Moniava, Dec 12 '14 at 14:43
@giorgim Yes, and this http://flamingdangerzone.com/cxx11/2012/08/15/rule-of-zero.html — Chris Drew, Dec 12 '14 at 16:30

Chris Drew · Answer 1 · 2014-12-12T16:04:53.630

Consider using std::unique_ptr<Item> and pass ownership from Main to the Manager:

#include <vector>
#include <memory>

class Item {};

class Manager {
    std::vector<std::unique_ptr<Item>> item_archive;
  public:
    void addItem(std::unique_ptr<Item> item){
        item_archive.push_back(std::move(item));
    }
};

int main() {
    Manager manager;
    auto item = std::make_unique<Item>();  // C++14
    //auto item = std::unique_ptr<Item>(new Item);  // C++11
    manager.addItem(std::move(item));
}

This way, the items will be deleted when the Manager is destroyed without you having to write a destructor.

I suggest using std::vector instead of an array because it simplifies the management of the item archive.

It is impossible to tell what exactly is giving you the runtime error as you have not posted real code but using std::vector and std::unique_ptr instead will most likely fix it.

Live demo

Thanks for the example,i think your post put me in the right direction.Now i have some errors like "attempting to reference a deleted function" but i'm trying to sort it out. — MTKJacob, Dec 12 '14 at 16:07
To work with `std::unique_ptr` you need to have some understanding of [move semantics](http://stackoverflow.com/questions/3106110/what-is-move-semantics) as `std::unique_ptr` is movable but not copyable (hence the errors). But it is worth the effort. — Chris Drew, Dec 12 '14 at 16:12

noelicus · Answer 2 · 2014-12-12T15:27:29.517

-1

There's no reason that Manager is not allowed to delete the Items. Your error could be that you're exceeding the ItemArchive array or deleting objects twice? It's hard to tell since you've re-written the code for the question.

I'd say try and use a system that you can debug and then you'll learn a lot quicker! Debuggers are just amazing things.

edited Dec 12 '14 at 15:27

answered Dec 12 '14 at 14:38

noelicus

14,468
3
92
111

No this code is not fine, it's not even correct. You can not say what actually is working in this example. Obviously `AddItem` would not add more then one item, and `~Manager` would delete uninitialized values. – Arpegius Dec 12 '14 at 15:19
If `archiveLength` is correct it will only delete the single added value. It's "fine" in relation to his query, which is "can another class delete an object it didn't create". That's why I expanded on what "fine" meant with a colon. Learn to read. I've edited my answer so people like you don't have to worry about grammar etc. – noelicus Dec 12 '14 at 15:27

Correct way to store objects in another object in C++?

2 Answers2