Passing an abstract class reference to a shared_ptr?

Hi guys,

So I'm trying to kinda do a Java thing with C++ which is apparently capable. I'm trying to pass a reference of a base abstract class to a different class and then make it a shared_ptr, like this:

Code:

---

SimpleApplication.cpp:
  mp_stateManager = std::make_shared<managers::AppStateManager>(*this);

AppStateManager.cpp:
    AppStateManager(gyroInterface::GyroApplication& app) :
    mp_app(std::make_shared<gyroInterface::GyroApplication>(app) {
        //...
    }

---

GyroApplication is a pure abstract class.

When I run this code in Netbeans I get the following error:

"note: in instantiation of function template specialization"

But when I do this:

Code:

---

    AppStateManager(gyroInterface::GyroApplication& app) {
        gyroInterface::GyroApplication* diffApp = &app;
    }

---

It works... Am I missing something? Or am I making a stupid mistake that new people tend to make?

Cheers Guys!

Jamie.

Just in case this is merely the result of a typo error (you only posted the start of the error message that is a preamble to identify where the error was detected), I note that you are missing a closing parenthesis, i.e., this:

Code:

---

    AppStateManager(gyroInterface::GyroApplication& app) :
    mp_app(std::make_shared<gyroInterface::GyroApplication>(app) {
        //...
    }

---

should have been:

Code:

---

    AppStateManager(gyroInterface::GyroApplication& app) :
    mp_app(std::make_shared<gyroInterface::GyroApplication>(app)) {
        //...
    }

---

Quote:

---

Originally Posted by laserlight

Just in case this is merely the result of a typo error (you only posted the start of the error message that is a preamble to identify where the error was detected), I note that you are missing a closing parenthesis, i.e., this:

Code:

---

    AppStateManager(gyroInterface::GyroApplication& app) :
    mp_app(std::make_shared<gyroInterface::GyroApplication>(app) {
        //...
    }

---

should have been:

Code:

---

    AppStateManager(gyroInterface::GyroApplication& app) :
    mp_app(std::make_shared<gyroInterface::GyroApplication>(app)) {
        //...
    }

---

---

They are typos, here's the full error:

Code:

---

note: in instantiation of function template specialization 'std::__1::make_shared<gyroEngine::gyroInterface::GyroApplication, gyroEngine::gyroInterface::GyroApplication &>' requested here
        mp_app = std::make_shared<gyroInterface::GyroApplication>(app);

---

That's not the full error either, still only the preamble. I suggest that you post the smallest and simplest version of this program that you expect to compile, but which demonstrates this error.

Quote:

---

Originally Posted by laserlight

That's not the full error either, still only the preamble. I suggest that you post the smallest and simplest version of this program that you expect to compile, but which demonstrates this error.

---

OK:

Code:

---

#include <memory>

// Abstract class
namespace interface {
    class MyInterface {
        public:
            MyInterface() {}
           
            virtual bool init() = 0;

            virtual int start() = 0;

            virtual bool cleanup() = 0;
    };
}

namespace manager {
    class Manager {
    private:
        std::shared_ptr<application::MyApplication> mp_app;

    public:
        Manager(application::MyApplication& app);
    };
}

namespace application {
    class MyApplication : public interface::MyInterface {
        private:
            std::shared_ptr<managers::Manager> mp_manager;
        public:
            MyApplication();
    };
}

manager::Manager(application::MyApplication& app) :
mp_app(std::make_shared<application::MyApplication>(app)) {}

application::MyApplication() {
    mp_manager = std::make_shared<manager::Manager>(*this);
}

int main() {
    application::MyApplication app();

    return 0;

---

That should do it.

This compiles with VS2017.

Code:

---

#include <memory>

namespace application {
        class MyApplication;
}

// Abstract class
namespace interface {
        class MyInterface {
        public:
                MyInterface() {}

                virtual bool init() = 0;

                virtual int start() = 0;

                virtual bool cleanup() = 0;
        };
}

namespace manager {
        class Manager {
        private:
                std::shared_ptr<application::MyApplication> mp_app;

        public:
                Manager(application::MyApplication& app);
        };
}

namespace application {
        class MyApplication : public interface::MyInterface {
        private:
                std::shared_ptr<manager::Manager> mp_manager;
        public:
                MyApplication();
                bool init() override { return true; }
                int start() override { return 0; }
                bool cleanup() override {return true; }
        };
}

manager::Manager::Manager(application::MyApplication& app) :
        mp_app(std::make_shared<application::MyApplication>(app)) {}

application::MyApplication::MyApplication() {
        mp_manager = std::make_shared<manager::Manager>(*this);
}

int main() {
        application::MyApplication app;

        return 0;
}

---

Quote:

---

Originally Posted by 2kaud

This compiles with VS2017.

Code:

---

#include <memory>

namespace application {
        class MyApplication;
}

// Abstract class
namespace interface {
        class MyInterface {
        public:
                MyInterface() {}

                virtual bool init() = 0;

                virtual int start() = 0;

                virtual bool cleanup() = 0;
        };
}

namespace manager {
        class Manager {
        private:
                std::shared_ptr<application::MyApplication> mp_app;

        public:
                Manager(application::MyApplication& app);
        };
}

namespace application {
        class MyApplication : public interface::MyInterface {
        private:
                std::shared_ptr<manager::Manager> mp_manager;
        public:
                MyApplication();
                bool init() override { return true; }
                int start() override { return 0; }
                bool cleanup() override {return true; }
        };
}

manager::Manager::Manager(application::MyApplication& app) :
        mp_app(std::make_shared<application::MyApplication>(app)) {}

application::MyApplication::MyApplication() {
        mp_manager = std::make_shared<manager::Manager>(*this);
}

int main() {
        application::MyApplication app;

        return 0;
}

---

---

What I would like to do is have a generic state manager which takes the interface as a parameter but give it the myapplication class, if that makes sense, that doesn't seem to compile in Netbeans 8.1

Quote:

---

Originally Posted by Jamie_Edwards

That should do it.

---

Like 2kaud in #6 I got it to compile but I think there are quite a few issues.

I suggest you put in a virtual destructor in MyInterface. Otherwise there will be undefined behavior if for some reason a pointer to MyInterface gets deleted. It may never happen but it's a good precaution. All derived classes of MyInterface will then automatically have virtal destructors too.

The reason for the compiler errors may have been the recursive definitions of Manager and MyApplication (each use the other). This can be resolved for the time being by a forward declaration (as 2kaud did with Myapplication) but as you start adding code new problems may surface. Even though this also can be overcome, recursive definitions are not good from a design standpoint. I suggest you instead make the recursive definition go away by a re-design. It's called de-coupling and in this case you really need to do it because Manager and MyApplication refer to each other by shared_ptr which cannot handle circular referencing so it's a bug.

If you plan to handle objects of a certain class by shared_ptr I recommend you never ever create and pass around them in any other form than as a shared_ptr. You are doing that now. For example you are passing a MyApplication object to a Manager constructor which then makes a shared_ptr. Instead a shared_ptr<MyApplication> should be passed in.

Furthermore when you make the shared_ptr<MyApplication> in Manager you do,

Code:

---

mp_app(std::make_shared<application::MyApplication>(app)) {}

---

Here make_shared will make a new MyApplication object on the heap. Fine, by why do you pass in app? It will be copy constructed and then just discarded so mp_app will end up holding a brand new MyApplication object and not app. But note that as it stands you cannot just leave out app and do this,

Code:

---

mp_app(std::make_shared<application::MyApplication>(/* app omitted */)) {}

---

because then instead the default constructor of MyApplication will be called (rather than the copy constructor) and you have infinite recursion on your hands. So app is nothing but a dummy. It happens to avoid infinite recursion as a side effect but it's a very obscure and convoluted way to achieve that. To do it properly the Manager and MyApplication classes shouldn't be recursively defined in the first place.

Also note there's another danger lurking here. Lets say app got copied and got used. Since app is a polymorphic object it could then have been sliced and that would've been a hard to find bug. Therefore a common precaution is to disable copying of polymorphic classes, that is to delete the copy constructor and the assignment operator..

Another dubious practice is this,

Code:

---

        mp_manager = std::make_shared<manager::Manager>(*this);

---

The problem is not what you do but that you do it in a constructor in the process of constructing the *this object. It's not a good idea to use an object until it's fully constructed. It's especially dangerous in concurrency situations but it's safer to never do it.

I suggest you rethink your design. A summary of my advice is,

1. Always use a virtual destructor in a polymorphic base class.

2. If you've decided to handle objects of a class by shared_ptr never handle them in any other way.

3. If you use shared_ptr be careful not to build circular reference chains.

4. Don't use an object until it's fully constructed. A constructor shouldn't "leak" the object it's constructing.

5. Avoid tight coupling of classes.

6. Avoid the "slicing problem" by disabling copying of polymorphic objects. Instead copy such objects by way of cloning.

Quote:

---

Like 2kaud in #6 I got it to compile but I think there are a quite few issues.

---

Yeah. I only fixed the c++ syntax errors - I don't sort out the design issues. :)