tenspd137
tenspd137

Reputation: 393

Is it possible to use std::make_unique in a class constructor and `this` pointer?

I am trying to use unique_ptr instead of raw pointers for components that are assigned to an actor class.

#include <iostream>
#include <memory>
#include <utility>
#include <algorithm>
#include <vector>

class Component;

class Actor
{
public:
        void AddComponent(std::unique_ptr<Component> component)
        {
                std::cout << "Add component\n";
                mComponents.push_back(std::move(component));
        }

        void RemoveComponent(Component* component)
        {
                std::cout << "Removing Component...\n";
                auto iter = std::find_if(mComponents.begin(), mComponents.end(),[&](const auto& e){ return e.get() == component;});
                if(iter != mComponents.end())
                {
                        mComponents.erase(iter);
                        std::cout << "Removed from vector \n";
                }
        }

        void ListComponents()
        {
                for(const auto& element : mComponents)
                {
                    std::cout << "Component:" << element.get() << "\n";
                }
        }
private:
        std::vector<std::unique_ptr<Component> > mComponents;
};

class Component
{
public:
        Component(Actor* actor) : mOwner(actor) {actor->AddComponent(std::make_unique<Component>(*this));}
        virtual ~Component() { std::cout << this << ":Destroyed \n"; }
private:
        Actor* mOwner = nullptr;
};

int main(int argc, char* argv[])
{
        Actor Actor1;
        auto c1 = Component(&Actor1);
        auto c2 = Component(&Actor1);
        Actor1.ListComponents();
}

Is there a way to assign the this pointer to a unique_ptr, or am I right that I need to use a factory function of some kind?

Thanks!

EDIT: Corrected Code:

#include <iostream>
#include <memory>
#include <utility>
#include <algorithm>
#include <vector>

class Component;

class Actor
{
public:
        void AddComponent(std::unique_ptr<Component> component)
        {
                std::cout << "Add component\n";
                mComponents.push_back(std::move(component));
        }

        void RemoveComponent(Component* component)
        {
                std::cout << "Removing Component...\n";
                auto iter = std::find_if(mComponents.begin(), mComponents.end(),[&](const auto& e){ return e.get() == component;});
                if(iter != mComponents.end())
                {
                        mComponents.erase(iter);
                        std::cout << "Removed from vector \n";
                }
        }

        void ListComponents()
        {
                for(const auto& element : mComponents)
                {
                    std::cout << "Component:" << element.get() << "\n";
                }
        }
private:
        std::vector<std::unique_ptr<Component> > mComponents;
};

class Component
{
public:
        Component(Actor* actor) : mOwner(actor) {actor->AddComponent(std::unique_ptr<Component>(this));}
        virtual ~Component() { std::cout << this << ":Destroyed \n"; }
private:
        Actor* mOwner = nullptr;
};

int main(int argc, char* argv[])
{
        Actor Actor1;
        auto c1 = new Component(&Actor1);
        auto c2 = new Component(&Actor1);
        Actor1.ListComponents();
}

Upvotes: 0

Views: 767

Answers (1)

AProgrammer
AProgrammer

Reputation: 52294

There are at least three issues here.

  1. There are various syntax errors (missing declaration of Component before the definition of Actor, typos, ... I'll ignore them now they are mentionned.

  2. The if you have any pointer, you can build a unique_ptr from it which will handle the lifetime of the object. That's not made with make_unique but with the constructor:

    owner->AddComponent(std::unique_ptr<Component>(this));
    
  3. You can do that but the unique_pointer will manage the lifetime of the Component. So that means that nothing else should and you have to avoid that, especially temporaries and stack allocated one. So you'll have to replace:

    c1 = Component(&Actor1);
    c2 = Component(&Actor1);
    

    by something like:

    new Component(&Actor1);
    new Component(&Actor1);
    

    There are various way to enforce that so that mistakes are detected at compilation, but I'll not go into them.

Upvotes: 1

Related Questions