boost-ext / di

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

why raw pointer policy is only unique and transfer ownership? #427

Closed davidtazy closed 4 years ago

davidtazy commented 5 years ago

Hello, The 2 lines in the main of the tutorial are mind blowing!!!

I just watch ur cppnow2019 video - and i loved it. You show a lot of best practices. SOLID, low coupling...

So i'm very surprised that raw pointer policy is 'unique' only. which is against cppCoreGuideline I.11 .

Every object passed as a raw pointer (or iterator) is assumed to be owned by the caller, so that its lifetime is handled by the caller. Viewed another way: ownership transferring APIs are relatively rare compared to pointer-passing APIs, so the default is "no ownership transfer."

It is bother me because I do DI manually like that:

int main()
{
  moduleA a;
  moduleB b(&a);
  moduleC c(&a);

  return c.run();
}

Furthermore using unique_ptr and shared_pointer in the code is noisy when reading and writing. I prefer use them for object with a life time shorter then the main.

So is it a technical problem or a philosophic one? Do you think "my code style" is broken?

thx u

tli2 commented 5 years ago

I am also interested in any workarounds to this issue. I work in a codebase where only const T & and T * are allowed (for clarity on mutability of parameters passed). Thus, in the codebase our constructors frequently take in T * for singleton scoped values.

So far, I have tried to write a custom scope to be able to inject singletons for pointers (we will manually declare scopes for types, so no change in deduction required). I tried working with the example shown in the tutorial with shared_ptrs and just changing them to raw pointers. That did not work.

Is defining a custom scope the right workaround here, and if so, are there any documentation or tutorial in how to make it work?

kanstantsin-chernik commented 4 years ago

You can do it with your own scope and config. I still believe that it is not the best idea. But here is an example:

//
// Copyright (c) 2012-2019 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)
//
#pragma once

#include <memory>
#include <typeindex>
#include <unordered_map>
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
#include <mutex>
#endif

#include "boost/di.hpp"

BOOST_DI_NAMESPACE_BEGIN
namespace extension {

namespace wrappers {
template <class TScope, class T, class TObject = std::shared_ptr<T>>
struct ptr_shared {
  using scope = TScope;
  template <class>
  struct is_referable_impl : aux::true_type {};
  template <class I>
  struct is_referable_impl<std::shared_ptr<I>> : aux::is_same<I, T> {};
  template <class I>
  struct is_referable_impl<boost::shared_ptr<I>> : aux::false_type {};
  template <class T_>
  using is_referable = is_referable_impl<aux::remove_qualifiers_t<T_>>;
  template <class I, __BOOST_DI_REQUIRES(aux::is_convertible<T*, I*>::value) = 0>
  inline operator std::shared_ptr<I>() const noexcept {
    return object;
  }
  inline operator std::shared_ptr<T>&() noexcept { return object; }
  template <class I, __BOOST_DI_REQUIRES(aux::is_convertible<T*, I*>::value) = 0>
  inline operator boost::shared_ptr<I>() const noexcept {
    struct sp_holder {
      std::shared_ptr<T> object;
      void operator()(...) noexcept { object.reset(); }
    };
    return {object.get(), sp_holder{object}};
  }
  template <class I, __BOOST_DI_REQUIRES(aux::is_convertible<T*, I*>::value) = 0>
  inline operator std::weak_ptr<I>() const noexcept {
    return object;
  }
  inline operator T&() noexcept { return *object; }
  inline operator const T&() const noexcept { return *object; }
  inline operator T*() noexcept { return object.get(); }
  inline operator const T*() const noexcept { return object.get(); }
  TObject object;
};

template <class TScope, class T>
struct ptr_shared<TScope, T&> {
  using scope = TScope;
  template <class>
  struct is_referable : aux::true_type {};
  explicit ptr_shared(T& object) : object(&object) {}
  template <class I>
  explicit ptr_shared(I);
  template <class I, __BOOST_DI_REQUIRES(aux::is_convertible<T, I>::value) = 0>
  inline operator I() const noexcept {
    return *object;
  }
  inline operator T&() const noexcept { return *object; }
  inline operator T*() const noexcept { return object; }
  T* object = nullptr;
};
}  // namespace wrappers

namespace detail {
class ptr_shared {
 public:
  template <class, class T>
  class scope {
   public:
    scope() noexcept = default;

#if !defined(BOOST_DI_NOT_THREAD_SAFE)
    //<<lock mutex so that move will be synchronized>>
    explicit scope(scope&& other) noexcept : scope(std::move(other), std::lock_guard<std::mutex>(other.mutex_)) {}
    //<<synchronized move constructor>>
    scope(scope&& other, const std::lock_guard<std::mutex>&) noexcept : object_(std::move(other.object_)) {}
#endif

    template <class T_, class>
    using is_referable = typename wrappers::ptr_shared<ptr_shared, T>::template is_referable<T_>;

    template <class, class, class TProvider>
    static auto try_create(const TProvider& provider)
        -> decltype(wrappers::ptr_shared<ptr_shared, T>{std::shared_ptr<T>{provider.get()}});

    /**
     * `in(ptr_shared)` version
     */
    template <class, class, class TProvider>
    wrappers::ptr_shared<ptr_shared, T> create(const TProvider& provider) & {
      if (!object_) {
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
        std::lock_guard<std::mutex> lock(mutex_);
        if (!object_)
#endif
          object_ = std::shared_ptr<T>{provider.get()};
      }
      return wrappers::ptr_shared<ptr_shared, T>{object_};
    }

    /**
     * Deduce scope version
     */
    template <class, class, class TProvider>
    wrappers::ptr_shared<ptr_shared, T> create(const TProvider& provider) && {
      auto& object = provider.cfg().template data<T>();
      if (!object) {
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
        std::lock_guard<std::mutex> lock(mutex_);
        if (!object)
#endif
          object = std::shared_ptr<T>{provider.get()};
      }
      return wrappers::ptr_shared<ptr_shared, T>{std::static_pointer_cast<T>(object)};
    }

   private:
#if !defined(BOOST_DI_NOT_THREAD_SAFE)
    std::mutex mutex_;
#endif
    std::shared_ptr<T> object_;  /// used by `in(ptr_shared)`, otherwise destroyed immediately
  };
};
}  // namespace detail

static constexpr detail::ptr_shared ptr_shared{};

class ptr_shared_config : public di::config {
  template <class T>
  struct type {
    static void id() {}
  };
  template <class T>
  static auto type_id() {
    return reinterpret_cast<std::size_t>(&type<T>::id);
  }

 public:
  template <class T>
  struct scope_traits {
    using type = typename di::config::scope_traits<T>::type;
  };

  template <class T>
  struct scope_traits<T&> {
    using type = detail::ptr_shared;
  };
  template <class T>
  struct scope_traits<T*> {
    using type = detail::ptr_shared;
  };

  template <class T>
  struct scope_traits<std::shared_ptr<T>> {
    using type = detail::ptr_shared;
  };

  template <class T>
  struct scope_traits<boost::shared_ptr<T>> {
    using type = detail::ptr_shared;
  };

  template <class T>
  struct scope_traits<std::weak_ptr<T>> {
    using type = detail::ptr_shared;
  };

  template <class T>
  auto& data() {
    return data_[type_id<T>()];
  }

 private:
  std::unordered_map<std::size_t, std::shared_ptr<void>> data_{};
};

}  // namespace extension
BOOST_DI_NAMESPACE_END

Thank you can use it like this:

      auto* test2 = injector.create<implementation2*>();
      auto* test1 = injector.create<implementation2*>();
      assert(test1 == test2);