boost-ext / di

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

A module with explicitly exposed types does not inject values bound outside the module #332

Closed jneuhaus20 closed 6 years ago

jneuhaus20 commented 6 years ago

Expected Behavior

A module with an exposed type should inject values bound outside the module (e.g. in the parent injector.) This works correctly when the module is declared with auto.

Actual Behavior

A default constructed object is injected instead.

Code Example

Slightly modified from the Module exposed type example below. This fails the assert at assert(42 == object.i); I've tried this with msvc in Visual Studio 15.6.4 and in the Try Online

//
// Copyright (c) 2012-2018 Kris Jusiak (kris at jusiak dot net)
//
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
//
//<-
#include <boost/di.hpp>
#include <cassert>
#include <memory>

namespace di = boost::di;

struct i1 {
  virtual ~i1() noexcept = default;
  virtual void dummy1() = 0;
};
struct i2 {
  virtual ~i2() noexcept = default;
  virtual void dummy2() = 0;
};
struct impl1 : i1 {
  void dummy1() override {}
};
struct impl2 : i2 {
  void dummy2() override {}
};
//->

struct T {
  T(std::shared_ptr<i1> i1, std::shared_ptr<i2> i2, int i) : i1_(i1), i2_(i2), i(i) {}

  std::shared_ptr<i1> i1_;
  std::shared_ptr<i2> i2_;
  int i;
};

di::injector<T> module() noexcept {
  // clang-format off
  return di::make_injector(
    di::bind<i1>().to<impl1>()
  , di::bind<i2>().to<impl2>()
  );
  // clang-format on
}

int main() {
  auto injector = di::make_injector(
    di::bind<int>().to(42),
    module());
  auto object = injector.create<T>();
  assert(dynamic_cast<impl1*>(object.i1_.get()));
  assert(dynamic_cast<impl2*>(object.i2_.get()));
  assert(42 == object.i);
}
kris-jusiak commented 6 years ago

Thanks @jneuhaus20. For the exposed module to work properly you would need to bind int inside the module itself not outside (the module is type erased with all bindings).

di::injector<T> module() noexcept {
  // clang-format off
  return di::make_injector(
    di::bind<i1>().to<impl1>()
  , di::bind<i2>().to<impl2>()
  , di::bind<int>().to(42)
  );
  // clang-format on
}

Full example here -> https://wandbox.org/permlink/bnbfTeNWp1TD3IEk

jneuhaus20 commented 6 years ago

So an exposed module can never be injected into? It works perfectly fine with auto, though, which is confusing. It should also probably be called out in the docs if it’s a design decision.

For my purposes, I don’t care about limiting the exposed types per se, so I’m open to another route. Are there any other tricks I can use to define modules in a cpp, or make nested dynamic bindings more manageable?

kris-jusiak commented 6 years ago

It's hard to say without seeing a full picture but there are a few options.

  1. Pass a value to a module

    di::injector<T> module(int&& i) noexcept {
    // clang-format off
    return di::make_injector(
    di::bind<i1>().to<impl1>()
    , di::bind<i2>().to<impl2>()
    , di::bind<int>().to(i)
    );
    // clang-format on
    }
    auto injector = di::make_injector(module(42));

    Full example here -> https://wandbox.org/permlink/cbZZuwNZcf0RFjyD

  2. Pass another exposed module to a module

    
    di::injector<int> int_module() noexcept {
    return di::make_injector(
    di::bind<int>.to(42)
    );
    }

di::injector module(di::injector&& im) noexcept { // clang-format off return di::make_injector( di::bind().to() , di::bind().to() , std::move(im) ); // clang-format on }

```cpp
  auto injector = di::make_injector(module(int_module()));

Full example here -> https://wandbox.org/permlink/AMLaom9UUNy7HKej

jneuhaus20 commented 6 years ago

Thanks for the tips @krzysztof-jusiak! I'm composing modules and returning subclasses of a common base, and since I didn't want to maintain the exposed types (or risk missing one) I went with passing modules in and having my outer function return by base class ptr. Something like:

template<typename InjectorType>
std::unique_ptr<Base> route(InjectorType baseInjector, StructWithInfo s) {
  try {
    auto newInjector = appendInjectors<ForThis, ForThat>(std::move(baseInjector), s);

  // Switch in a different func, omitted for brevity
    switch (s.fancyEnum) {
      case A: return newInjector.create<SpecificType1>();
      case B: return newInjector.create<SpecificType2>();
      default:  throw std::exception("woops");
    }
  } catch (std::exception& e) {
    return std::make_unique<ErrorType>(e.what);
  }
}

And the helpers looked like this:

class ForThis {
public:
  template<typename InjectorType>
  static auto add(InjectorType injector, StructWithInfo s) {
    if (s.thisValid== false)
      throw std::exception("This wasn't valid");

    return di::make_injector(
      std::move(injector)
    , di::bind<iThis>().to<implThis>()
    );
  }
};

Made extensible with this:

// Tail case to stop recursion
template<typename InjectorType>
auto appendInjectors(InjectorType injector, StructWithInfo)
{
    return std::move(injector);
}

// Specify one more more appending policies and let InjectorType be inferred
template<typename NextAppender, typename ...Rest, typename InjectorType>
auto appendInjectors(InjectorType injector, StructWithInfo s)
{
    return appendInjectors<Rest...>(NextAppender::convert(std::move(injector), s), s);
}

It was an interesting paradigm that helped me a bunch, so I figured I'd share. I had trouble getting an example to work in Wandbox, but the production code works in msvc 2017, and that's what I need so I stopped messing with it 😛