Implementation of a C++ IOC container

Question

I have created a pretty simple IOC container for C++ that should allow me to write less coupled code in the future and make my code easier to unit test.

Overall I am fairly happy however I feel like I could probably include some additional functionality to improve the code further.

Any suggestions on how the code could be improved or any noticeable issues are much appreciated.

In the real production code the implementation will be namespaced etc. but I left it out for sake of example. I understand that singleton pattern is generally frowned upon however I do think it serves a purpose here.

Worth noting I cannot use std::unique_ptr due to real implementation using boost instead of c++11.

Implementation(IOC.hpp)

class IOCContainer
{
    public:
        static IOCContainer& Instance()
        {
            static IOCContainer instance;
            return instance;
        }

        ///
        /// Pass in a shared pointer of class T and we will automatically deduce the type using the typeid of T
        ///
        template<class T>
        void Register(std::shared_ptr<T> t)
        {
            const std::type_info* typeId = &typeid(T);

            Register(typeId->raw_name(), t);
        }

        ///
        /// Pass in a shared pointer as well as a type id. This is useful for registering a base class/interface
        /// alongside a derived class shared pointer i.e. pass in &typeid(Base) and boost::shared_ptr<Derived>
        ///
        template<class T>
        void Register(const std::type_info* typeId, std::shared_ptr<T> t)
        {
            if (!typeId)
            {
                throw std::runtime_error("Invalid type id to register");
            }

            Register(typeId->raw_name(), t);
        }

        ///
        /// Pass in a shared pointer as well as a custom id. This can be useful if we wish to use an interface more than once
        /// with different base classes
        ///
        template<class T>
        void Register(const std::string& id, std::shared_ptr<T> t)
        {
            std::lock_guard<std::mutex> lock(m_mapMutex);

            std::map<std::string, std::shared_ptr<void> >::iterator iter = m_mapping.find(id);

            if (iter == m_mapping.end())
            {
                m_mapping[id] = t;
            }
        }

        ///
        /// Resolves the pointer type based on the type id of the supplied T
        /// 
        template<class T>
        std::shared_ptr<T> Resolve()
        {
            const std::type_info* typeId = &typeid(T);

            return Resolve<T>(typeId->raw_name());
        }

        ///
        /// Resolves the pointer type based on the supplied type id
        /// 
        template<class T>
        std::shared_ptr<T> Resolve(const std::type_info* typeId)
        {
            if (!typeId)
            {
                throw std::runtime_error("Invalid type id to resolve");
            }

            return Resolve<T>(typeId->raw_name());
        }

        ///
        /// Resolves the type based on the supplied ID
        ///
        template<class T>
        std::shared_ptr<T> Resolve(const std::string& id)
        {
            std::lock_guard<std::mutex> lock(m_mapMutex);

            std::map<std::string, std::shared_ptr<void> >::iterator iter = m_mapping.find(id);

            if (iter != m_mapping.end())
            {
                return std::static_pointer_cast<T>(iter->second);
            }

            throw std::runtime_error("Could not locate type in IOC");
        }
    private:
        std::map<std::string, std::shared_ptr<void> > m_mapping;
        std::mutex m_mapMutex;

        // Disable construction
        IOCContainer(){}
        // Disallow copying of object to ensure we don't get accidental copies of our IOCContainer
        IOCContainer(const IOCContainer&);
        void operator=(const IOCContainer&);
    };

Test Code

#include "IOC.hpp"

#include <iostream>
#include <string>
#include <memory>

class A
{
public:
    A(std::string stuff) : m_stuff(stuff){}
    virtual ~A(){}
    virtual std::string GetStuff() const { return m_stuff; }
private:
    std::string m_stuff;
};

class C : public A
{
public:
    C(std::string stuff) : A(stuff)
    {

    }
};

class B
{
public:
    B() : 
        m_default(IOCContainer::Instance().Resolve<C>()),
        m_baseTest(IOCContainer::Instance().Resolve<A>()),
        m_baseTest2(IOCContainer::Instance().Resolve<A>(&typeid(A))),
        m_idTest1(IOCContainer::Instance().Resolve<A>("IdTest")),
        m_idTest2(IOCContainer::Instance().Resolve<A>("IdTest2"))
    {
        std::cout << m_default->GetStuff() << std::endl;
        std::cout << m_baseTest->GetStuff() << std::endl;
        std::cout << m_baseTest2->GetStuff() << std::endl;
        std::cout << m_idTest1->GetStuff() << std::endl;
        std::cout << m_idTest2->GetStuff() << std::endl;
    }
private:
    std::shared_ptr<C> m_default;
    std::shared_ptr<A> m_baseTest;
    std::shared_ptr<A> m_baseTest2;
    std::shared_ptr<A> m_idTest1;
    std::shared_ptr<A> m_idTest2;
};

int main()
{
    IOCContainer::Instance().Register(std::make_shared<C>("DefaultTest"));
    IOCContainer::Instance().Register(&typeid(A), std::make_shared<C>("BaseTest"));
    IOCContainer::Instance().Register("IdTest", std::make_shared<C>("IdTest"));
    IOCContainer::Instance().Register("IdTest2", std::make_shared<C>("IdTest2"));

    B b;
}

Answer 1

Just some ideas about implementation.

disabling methods

You might consider deleted functions (=delete) feature of C++11.

http://en.cppreference.com/w/cpp/language/function#Deleted_functions

    // Disable construction
    IOCContainer() = delete;
    // Disallow copying of object to ensure we don't get accidental copies of our IOCContainer
    IOCContainer(const IOCContainer&) = delete;
    void operator=(const IOCContainer&) = delete;

std::type_info::name

I would be a little careful with this.

http://www.cplusplus.com/reference/typeinfo/type_info/name/

The particular representation pointed by the returned value is implementation-defined, and may or may not be different for different types.

Standard does not ask compilers to produce different names for different types so it is implementation defined. Compilers are probably going to implement it in a sensible manner.

pointer error handling

You might consider changing pointer to reference as you don't need to think about null_ptr here

    ///
    /// Pass in a shared pointer as well as a type id. This is useful for registering a base class/interface
    /// alongside a derived class shared pointer i.e. pass in typeid(Base) and boost::shared_ptr<Derived>
    ///
    template<class T>
    void Register(const std::type_info& typeId, std::shared_ptr<T> t)
    {
        Register(typeId.raw_name(), t);
    }

and here

    ///
    /// Resolves the pointer type based on the supplied type id
    /// 
    template<class T>
    std::shared_ptr<T> Resolve(const std::type_info& typeId)
    {
        return Resolve<T>(typeId->raw_name());
    }

duplicate registration

Is it intentional that in case specific type has already been registered you silently do nothing during second registration? I would consider throwing some exception.

    template<class T>
    void Register(const std::string& id, std::shared_ptr<T> t)
    {
        std::lock_guard<std::mutex> lock(m_mapMutex);

        std::map<std::string, std::shared_ptr<void> >::iterator iter = m_mapping.find(id);

        if (iter == m_mapping.end())
        {
            m_mapping[id] = t;
        }
    }

synchronization

You are probably locking too much. Simultaneous lookup is definitely not a problem. You might like some ideas about readers/writers locks or shared mutexes.

See e. g. these questions:

• https://stackoverflow.com/questions/16774469/a-rw-lock-for-c11-threads
• https://stackoverflow.com/questions/14306797/c11-equivalent-to-boost-shared-mutex