HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Implementation of a C++ IOC container

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationioccontainer

Problem

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
void Register(std::shared_ptr 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
///
template
void Register(const std::type_info* typeId, std::shared_ptr 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
void Register(const std::string& id, std::sha

Solution

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
    ///
    template
    void Register(const std::type_info& typeId, std::shared_ptr t)
    {
        Register(typeId.raw_name(), t);
    }


and here

///
    /// Resolves the pointer type based on the supplied type id
    /// 
    template
    std::shared_ptr Resolve(const std::type_info& typeId)
    {
        return Resolve(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
    void Register(const std::string& id, std::shared_ptr t)
    {
        std::lock_guard lock(m_mapMutex);

        std::map >::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

Code Snippets

// 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;
///
    /// 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);
    }
///
    /// 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());
    }
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;
        }
    }

Context

StackExchange Code Review Q#132821, answer score: 3

Revisions (0)

No revisions yet.