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

Proper use of type_info in relation to mapping components to a type

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

Problem

I'm trying to map a components type to the component value itself in a map. The following code all works properly. My main question is this. Is storing the map key as std::type_info* the best way to accomplish what I'm doing? I'm also not digging the ugly static_cast I'm doing during the usage part.

Entity.h

#pragma once
#include "IComponent.h"
#include 
#include 

class Entity
{
public:
    Entity(void);
    ~Entity(void);
    void AddComponent(IComponent* component);
    IComponent* GetComponent(const std::type_info* typeInfo);
private:
    std::map components;
};


Entity.cpp

#include "Entity.h"

Entity::Entity(void)
{
}

Entity::~Entity(void)
{
    for (auto &mapValue : components)
    {
        delete mapValue.second;
    }
}

void Entity::AddComponent(IComponent* component)
{
    const type_info* info = &typeid(*component);
    components.insert (std::pair(info, component));
}

IComponent* Entity::GetComponent(const std::type_info* typeInfo)
{
    return components.at(typeInfo);
}


Example Usage

IComponent* baseComponent = entity->GetComponent(&typeid(PerkList));
PerkList* perkList = static_cast(baseComponent);

Solution

type_info:

Using the type_info reference is a viable approach, but it can be made better with type_index in conjunction with an unordered_map.

static_cast:

I don't think there is a way around the cast here. You can make it slightly less verbose by defining a template helper method that wraps the type cast:

template
T * Entity::GetComponent()
{
    return static_cast(components.at(&typeid(T)));
}

// Usage:
PerkList * perkList = entity->GetComponent();


One nice extra of this approach is that you can replace the static_cast inside with a dynamic_cast during debug/dev builds and check that the types are self-consistent. Then add a preprocessor to use the faster static_cast for a "release" build if you need to squeeze every cycle.

Pointers:

Returning a raw pointer to IComponent is questionable to say the least. A raw pointer conveys no ownership, so it is very easy to leak or delete the objects multiple times when you have raw pointers flying around like this. You should take a look at shared_ptr and weak_ptr before further expanding your implementation. Or perhaps return just a reference (IComponent &) if the caller of GetComponent() should not own the object.

Minor details:

-
Don't add void to the parameter list of functions/methods that take no parameters. This is a C-ish style. C++ does not require that; () is just as good. So avoid the unnecessary verbosity.

-
Empty destructors/constructors should be omitted or made defaults.

Code Snippets

template<class T>
T * Entity::GetComponent()
{
    return static_cast<T *>(components.at(&typeid(T)));
}

// Usage:
PerkList * perkList = entity->GetComponent<PerkList>();

Context

StackExchange Code Review Q#84176, answer score: 6

Revisions (0)

No revisions yet.