CODEWITHSUNDEEP
c++c++11c++14smart-pointers
Oleg
Oleg

Reputation: 1097

Return unique_ptr with abstract class inside

I am trying to encapsulate details of the Engine implementation class. To do that I am returning std::unique_ptr of abstract class (IEngine in my case) instead of Engine. But I could not do that due to the compile error. I could return raw reference and it works but is that possible with unique_ptr? Thanks in advance.

class IEngine
{
public:
    virtual ~IEngine() = default;

    virtual void Start() = 0;
};

class Engine : public IEngine
{
public:
    void Start() override {}
};

class Car
{
    std::unique_ptr<Engine> m_engine;

public:

    std::unique_ptr<IEngine>& Get() { return m_engine; } // Here is compile error
};

int main()
{
    Car lambo;
}

Upvotes: 0

Views: 202

Answers (2)

463035818_is_not_an_ai
463035818_is_not_an_ai

Reputation: 122575

I am trying to encapsulate details of the Engine implementation class.

Returning a non-const reference to a private member is rarely the right thing to do. In any case it is the opposite of data encapsulation. Once the caller has the reference they can do with it whatever they like. Returning a non const reference makes sense for convenience access methods like for example std::vector::operator[]. The purpose of std::vector::operator[] is not to hide the element from the caller. There are other ways to get your hands on it. Rather std::vector::operator[] is to make it more convenient to access elements. Encapsulation it is not.

It is also not clear why you want to return a unique_ptr from Get. When no transfer of ownership is desired no smart pointer needs to be returned.

I could return raw reference

Yes, thats perfectly fine:

#include <memory>

class IEngine
{
public:
    virtual ~IEngine() = default;

    virtual void Start() = 0;
};

class Engine : public IEngine
{
public:
    void Start() override {}
};

class Car
{
    std::unique_ptr<Engine> m_engine;

public:

    const IEngine& Get() { return *m_engine; } // Here is compile error
};

int main()
{
    Car lambo;
}

Upvotes: 1

Caleth
Caleth

Reputation: 62779

std::unique_ptr<IEngine> is a different type to std::unique_ptr<Engine>, so you are asking to return a reference to a temporary object.

std::unique_ptr uniquely owns the object it points to, so even if you removed the reference, it would be incorrect to create a std::unique_ptr<IEngine> from the existing std::unique_ptr<Engine> that you presumably want to leave unchanged.

You shouldn't be exposing std::unique_ptr here. I'm not really sure you should be exposing IEngine here. I'm also confused as to why you need the concrete Engine type in Car, but the outside world needs mutable access to a pointer to IEngine.

I would instead expect something like:

class Car
{
    std::unique_ptr<IEngine> m_engine;

public:

    void TurnIgnition() { m_engine->Start(); }
};

Upvotes: 1

Related Questions