current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 4 down vote favorite

I'm going to implement something like the Unity3D component model for my C++ game engine. Please, review my draft of the Component base class:

class Component
{
public:
    template<class T>
    static T* create(void)
    {
        if (Type<T>::id == 0)
            Type<T>::id = ++_idCounter;

        T* component = new T();
        component->_id = Type<T>::id;
        return component;
    }

    template<class T>
    inline bool is(void) const
    {
        return Type<T>::id == _id;
    }

protected:
    Component(void) {}

private:
    static unsigned int _idCounter;
    unsigned int _id;

    template<class T> struct Type { static unsigned int id; };
};

unsigned int Component::_idCounter = 0;
template<class T> unsigned int Component::Type<T>::id = 0;

It has some RTTI-like functionality, and I can use it this way:

class ComponentA: public Component {};
class ComponentB: public Component {};

...

ComponentA* a = Component::create<ComponentA>();
ComponentB* b = Component::create<ComponentB>();

std::cout << a->is<ComponentA>() << std::endl; // 1
std::cout << b->is<ComponentB>() << std::endl; // 1
std::cout << b->is<ComponentA>() << std::endl; // 0
std::cout << a->is<ComponentB>() << std::endl; // 0

And I have a question. How can I prevent any construction of any descendant of Component outside of Component::create?

share|improve this question
2  
Any particular reason why you’re rolling your own RTTI, when C++ has a perfectly good built in facility for that? –  microtherion Jun 20 '13 at 22:28
    
By “prevent any construction of any descendant of Component”, if you mean just those under your control, you can prevent that by declaring the constructors of your descendants private, with class Component as a friend. If you want to prevent constructions of descendants implemented by others, you’re SOL. –  microtherion Jun 20 '13 at 22:30
2  
If the performance of your code critically depends on the speed of type identity comparisons, I suspect that your design is rather fundamentally misguided. In most C++ code, typeid comparisons are incredibly rare. –  microtherion Jun 20 '13 at 23:53
2  
A bit of nitpicking: Leading underscores are in many cases reserved for the compiler/implementation. If I recall the rules correctly, _x (single underscore followed by lower-case) is reserved when in global scope, and __x, __X and _X (double underscore followed by anything and single underscore followed by upper-case) is always reserved, regardless of scope. Since it's pointless to memorize these rules, I think leading underscores are bad practice in general. Make them a suffix instead: idCounter_. –  Lstor Jun 21 '13 at 9:54
1  
@Lstor: The rules. stackoverflow.com/a/228797/14065. Summry: As you said "avoid the use of leading underscore". Even if you can remember the rules a lot of people don't get it correct. –  Loki Astari Jun 22 '13 at 18:26

1 Answer 1

up vote 8 down vote

Sorry, but this is simply not a good design.

Your reliance on memory-owning raw pointers means that the library is easy to use wrongly, and that the user will leak memory. This is a big no-no in modern C++. First, because it’s harmful and makes user code brittle and complex; and second, because it is unnecessary. Modern C++ gives you all the tools to avoid manual memory management.

Think carefully about ownership in your component model and choose an appropriate smart pointer. Since you control the object hierarchy, boost::intrusive_ptr may be the best match, but consider other alternatives which are less generic (such as std::unique_ptr).

Finally think about ways to avoid pointer use completely, if possible. In fact, the code you’ve shown doesn’t use runtime polymorphism at all so there is no use of pointers. Just handle objects directly.

Regarding your implementation of RTTI: Without knowing the detailed requirements of your object model it’s hard to comment here; however, you say that you don’t want to get the RTTI overhead. What makes you think C++ RTTI has more overhead than your solution? In fact the implementation you’ve shown is, if anything, less efficient than C++ RTTI because your use-case can be resolved at compile-time.

But let’s assume that you just forgot to make your classes polymorphic, and that the real code cannot resolve the type information at compile time. Then your RTTI mechanism stores redundant information and is still less efficient than C++’ built-in mechanism.

Finally, if you are afraid of RTTI overhead (and have data to back this up) then the real solution is not to roll your own system, it’s to avoid RTTI be redesigning your classes such that they have a uniform interface and don’t need to be distinguished at runtime. In fact, this is the idiomatic solution in C++, and RTTI is the exception rather than the norm.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.