boost-ext / di

C++14 Dependency Injection Library
https://boost-ext.github.io/di
1.16k stars 140 forks source link

Default scope doesn't recognize scope of a shared_ptr as singleton if a dynamic binding is used. #214

Closed llaszko closed 5 years ago

llaszko commented 8 years ago

Default scope doesn't recognize scope of a shared_ptr as singleton if a dynamic binding is used.

Consider following code:

#include <cstdlib>
#include <iostream>

#include <boost/di.hpp>

namespace di = boost::di;

using namespace std;

struct ITypeInterface
{
    virtual ~ITypeInterface() { };

    virtual void say_hallo() = 0;
};

struct TypeImplementation : ITypeInterface
{
    virtual void say_hallo() override
    {
        cout << "Hallo from " << this << "!" << endl;
    }
};

int main(int argc, char** argv) 
{
    auto injector 
            = di::make_injector(
                    boost::di::bind<ITypeInterface>().to([&](const auto& injector) -> shared_ptr<ITypeInterface>
                    {
                        return shared_ptr<ITypeInterface>(new TypeImplementation());
                    }));

    auto instance1 = injector.create<shared_ptr<ITypeInterface>>();
    instance1->say_hallo();

    auto instance2 = injector.create<shared_ptr<ITypeInterface>>();
    instance2->say_hallo();        

    return 0;
}

when compiled and executed the output prints out:

Hallo from 0x2482050!
Hallo from 0x24820a0!

I'd expect in such case the factory lambda to be invoked once and the result to be reused as a singleton. Additionally, it's impossible to enforce singleton scope for this registration. If binding is amended to:

...
    auto injector 
            = di::make_injector(
                    boost::di::bind<ITypeInterface>().in(di::singleton).to([&](const auto& injector) -> shared_ptr<ITypeInterface>
                    {
                        return shared_ptr<ITypeInterface>(new TypeImplementation());
                    }));
...

clang 3.7 complains about

../edgecases/main.cpp:35:31: warning: 'create<std::__1::shared_ptr<ITypeInterface>, 0>' is deprecated: creatable constraint not satisfied [-Wdeprecated-declarations]
    auto instance1 = injector.create<shared_ptr<ITypeInterface>>();
                              ^
../thirdparty/di/include/boost/di.hpp:2417:3: note: 'create<std::__1::shared_ptr<ITypeInterface>, 0>' has been explicitly marked deprecated here
  create
  ^
../edgecases/main.cpp:38:31: warning: 'create<std::__1::shared_ptr<ITypeInterface>, 0>' is deprecated: creatable constraint not satisfied [-Wdeprecated-declarations]
    auto instance2 = injector.create<shared_ptr<ITypeInterface>>();
                              ^
../thirdparty/di/include/boost/di.hpp:2417:3: note: 'create<std::__1::shared_ptr<ITypeInterface>, 0>' has been explicitly marked deprecated here
  create
  ^
../thirdparty/di/include/boost/di.hpp:891:2: error: inline function 'boost::di::v1_0_0::concepts::abstract_type<ITypeInterface>::is_not_bound::error' is not defined [-Werror,-Wundefined-inline]
 error(_ = "type is not bound, did you forget to add: 'di::bind<interface>.to<implementation>()'?");
 ^
../thirdparty/di/include/boost/di.hpp:887:41: note: used here
      return constraint_not_satisfied{}.error();
krzysztof-jusiak commented 8 years ago

Thanks @llaszko for reporting this issue. You are absolutely right about the behavior. Boost.DI design(v1.0.0) is not deducing (or even using) the scope in case when instance scope is used (binding using to method). The reason behind such a design was the idea that instances provided by users will have specified life time already, however, this approach has a lot of limitations and therefore I'm planning to allow scopes deduction and scope specification in those cases for v1.1.0 (to be released in May/June 2016).

The only 'hacky' solution I can think of for now (v1.0.0) would be to specify the scope yourself which is far from begin ideal tho :(

    auto injector = di::make_injector(
        di::bind<ITypeInterface>().to([] {
            static auto object = std::make_shared<TypeImplementation>();
            return object;
        }));

which will print...

Hallo from 0x997c30!
Hallo from 0x997c30!

I will keep this issue open as long as the proper solution won't be implemented.

kanstantsin-chernik commented 6 years ago

Actually, I have very similar issue: I need to impose extended (shared) scope for dynamic binding. @krzysztof-jusiak do you have any plans to add this?

jrunestone commented 6 years ago

Is this issue still active? I'm using the latest cpp14 branch but I suspect it's not been addressed yet. I'm loving this library, great job guys.

kanstantsin-chernik commented 5 years ago

It is addressed at shared_factory extension.