27

I'm currently re-designing my Entity System, for C++, and I have a lot of Managers. In my design, I have these classes, in order to tie my library together. I've heard a lot of bad things when it comes to "manager" classes, perhaps I'm not naming my classes appropriately. However, I have no idea what else to name them.

Most of the managers, in my library, are composed of these classes (although it does vary a little bit):

  • Container - a container for objects in the manager
  • Attributes - attributes for the objects in the manager

In my new design for my library, I have these specific classes, in order to tie my library together.

  • ComponentManager - manages components in the Entity System

    • ComponentContainer
    • ComponentAttributes
    • Scene* - a reference to a Scene (see below)
  • SystemManager - manages systems in the Entity System

    • SystemContainer
    • Scene* - a reference to a Scene (see below)
  • EntityManager - manages entities in the Entity System

    • EntityPool - a pool of entities
    • EntityAttributes - attributes of an entity (this will only be accessible to ComponentContainer and System classes)
    • Scene* - a reference to a Scene (see below)
  • Scene - ties all the managers together

    • ComponentManager
    • SystemManager
    • EntityManager

I was thinking of just putting all the container/pools in the Scene class itself.

i.e.

Instead of this:

Scene scene; // create a Scene

// NOTE:
// I technically could wrap this line in a createEntity() call in the Scene class
Entity entity = scene.getEntityManager().getPool().create();

It would be this:

Scene scene; // create a Scene

Entity entity = scene.getEntityPool().create();

But, I am unsure. If I were to do the latter, that would mean I would have a lot of objects and methods declared inside my Scene class.

NOTES:

  1. An entity system is simply a design that is used for games. It is composed of 3 major parts: components, entities, and systems. The components are simply data, which may be "added" to the entities, in order for the entities to be distinctive. An entity is represented by an integer. Systems contain the logic for an entity, with specific components.
  2. The reason I'm changing my design for my library, is because I think it can be changed quite a lot, I don't like the feel/flow to it, at the moment.
5

4 Answers 4

19

DeadMG is spot on on the specifics of your code but I feel like it misses a clarification. Also I don't agree with some of his recommendation that don't hold in some specific contexts like most high performance-videogame development for example. But he's globally right that most of your code is not useful right now.

As Dunk says, Manager classes are called like this because they "manage" things. "manage" is like "data" or "do", it's an abstract word that can contain almost anything.

I was mostly in the same place than you around 7 years ago, and I started to think that there was something wrong in my way of thinking because it was so much efforts to code to do nothing yet.

What I changed to fix this is to change the vocabulary I use in code. I totally avoid generic words (unless it's generic code, but it's rare when you're not making generic libraries). I avoid to name any type"Manager" or "object".

The impact is direct in code: it forces you to find the right word corresponding to the real responsibility of your type. If you feel like the type does several things (keep an index of Books, keep them alive, create/destroy books) then you need different types that each will have one responsibility, then you combine them in the code that uses them. Sometime I need a factory, sometime not. Sometime I need a registry, so I setup one (using standard containers and smart pointers). Sometime I need a system that is composed of several different sub system so I separate everything as much as I can so that each part do something useful.

Never name a type "manager" and make sure all your type have a single unique role is my recommendation. It might be hard to find names sometime, but it's one of the hardest thing to do in programming generally

8
  • If a couple dynamic_casts are killing your performance, then use the static type system- that's what it's for. It's really a specialized context, not a general one, to have to roll your own RTTI like LLVM did. Even then, they don't do this.
    – DeadMG
    Feb 12, 2013 at 23:39
  • @DeadMG I was not talking about this part in particular, but most high-performance games can't afford, for example, dynamic allocations through new or even other stuffs that are fine for programming in general. They are specific beasts for specific hardware which you can't generalize which is why "it depends" is a more general answer, in particular with C++. (nobody in game dev uses dynamic cast by the way, it's never useful until you have plugins, and even then it's rare).
    – Klaim
    Feb 13, 2013 at 14:50
  • @DeadMG I'm not using dynamic_cast, nor am I using an alternative to dynamic_cast. I implemented it in the library, but I couldn't be bothered taking it out. template <typename T> class Class { ... }; is actually for assigning IDs for different classes (namely custom components and custom systems). ID assignment is used to store components/systems in a vector, as a map may not bring the performance I need for a game. Feb 27, 2013 at 11:25
  • Well I didn't actually implement an alternative to dynamic_cast either, just a fake type_id. But, still I didn't want what type_id achieved. Feb 27, 2013 at 11:34
  • "find the right word corresponding to the real responsibility of your type" - though i totally agree with this, often there is not one single word (if any) that the developer community has agreed upon for a particular type of responsibility.
    – bytedev
    Apr 24, 2014 at 15:49
22

Well, I had a read through some of the code you linked to, and your post, and my honest summary is that the majority of it is basically completely worthless. Sorry. I mean, you have all of this code, but you haven't achieved anything. At all. I'm going to have to go into some depth here, so bear with me.

Let's start with ObjectFactory. Firstly, function pointers. These are stateless, the only useful stateless way to create an object is new and you don't need a factory for that. Statefully creating an object is what's useful and function pointers are not useful for this task. And secondly, every object needs to know how to destroy itself, not having to find that exact creating instance to destroy it. And in addition, you must return a smart pointer, not a raw pointer, for the exception and resource safety of your user. Your whole ClassRegistryData class is just std::function<std::unique_ptr<Base, std::function<void(Base*)>>()>, but with the aforementioned holes. It does not need to exist at all.

Then we have AllocatorFunctor- there are already Standard allocator interfaces provided- Not to mention that they're just wrappers on delete obj; and return new T;- which again, is already provided, and they won't interact with any stateful or custom memory allocators which exist.

And ClassRegistry is simply std::map<std::string, std::function<std::unique_ptr<Base, std::function<void(Base*)>>()>> but you wrote a class for it instead of using the existing functionality. It's the same, but completely needlessly global mutable state with a bunch of crappy macros.

What I'm saying here is that you've created some classes where you could replace the entire thing with functionality built from Standard classes, and then made them even worse by making them global mutable state and involving a bunch of macros, which is the worst thing you could possibly do.

Then we have Identifiable. It's a lot of the same story here- std::type_info already performs the job of identifying each type as a run-time value, and it doesn't require excessive boilerplate on the part of the user.

    template<typename T, typename Y>
    bool isSameType(const X& x, const Y& y) { return typeid(x) == typeid(y); }
    template <class Type, class T>
    bool isType(const T& obj)
    {
            return dynamic_cast<const Type*>(std::addressof(obj));
    }

Problem solved, but without any of the preceeding two hundred lines of macros and whatnot. This also marks your IdentifiableArray as nothing but std::vector but you have to use reference-counting even if unique ownership or non-ownership would be better.

Oh, and did I mention that Types contains a bunch of absolutely useless typedefs? You gain literally no advantage whatsoever over using the raw types directly, and lose a good chunk of readability.

Next up is ComponentContainer. You can't choose the ownership, you can't have separate containers for derived classes of various components, you can't use your own lifetime semantics. Delete without remorse.

Now, the ComponentFilter might actually be worth a little, if you refactored it a lot. Something like

class ComponentFilter {
    std::unordered_map<std::type_info, unsigned> types;
public:
    template<typename T> void SetTypeRequirement(unsigned count = 1) {
        types[typeid(T)] = count;
    }
    void clear() { types.clear(); }
    template<typename Iterator> bool matches(Iterator begin, Iterator end) {
        std::for_each(begin, end, [this](const std::iterator_traits<Iterator>::value_type& t) {
            types[typeid(t)]--;
        });
        return types.empty();
    }
};

This doesn't deal with classes derived from the ones you're trying to deal with, but neither did yours.

The rest of this is pretty much the same story. There is nothing here that could not be done much better without the use of your library.

How would you avoid managers in this code? Delete basically all of it.

4
  • 16
    It took me a while to learn, but the most reliable, bug-free pieces of code are the ones that aren't there.
    – Tacroy
    Feb 12, 2013 at 19:47
  • 1
    The reason I didn't use std::type_info is because I was trying to avoid RTTI. I mentioned the framework was aimed at games, which is basically the reason I'm not using typeid or dynamic_cast. Thanks for the feedback though, I appreciate it. Feb 13, 2013 at 5:54
  • Is this critics based on your knowledge of component-entity system game engines or is it a general C++ code review?
    – Den
    Feb 13, 2013 at 11:17
  • 1
    @Den I think more of a design issue than anything else. Feb 14, 2013 at 1:51
10

The problem with Manager classes is that a manager can do anything. You can't read the class name and know what the class does. EntityManager....I know it does something with Entities but who knows what. SystemManager...yep, it manages the system. That's very helpful.

My guess is that your manager classes probably do a lot of things. I'll bet if you were to segment those things into closely related functional pieces, then you'll probably be able to come up with class names related to the functional pieces that anybody can tell what they do simply by looking at the class name. This will make for a far easier to maintain and understand design.

2
  • 1
    +1 and not only can they do anything, on a long enough timeline they will. People view the "Manager" descriptor as an invitation to create kitchen-sink/swiss-army-knife classes. Feb 12, 2013 at 19:47
  • @Erik - Ah yes, I should have added that having a good class name is not only important because it tells someone what the class can do; but equally the class name also says what the class shouldn't do.
    – Dunk
    Feb 19, 2013 at 15:58
4

I actually don't think Manager is so bad in this context, shrug. If there's one place where I think Manager is forgivable, it would be for an entity-component system, since it is really "managing" the lifetimes of components, entities, and systems. At the very least that's a more meaningful name here than, say, Factory which doesn't make as much sense in this context since an ECS does genuinely do a bit more than create things.

I suppose you could use Database, like ComponentDatabase. Or you might just use Components, Systems, and Entities (this is what I personally use and mainly just because I like terse identifiers though it admittedly conveys even less information, perhaps, than "Manager").

The one part I find a bit clumsy is this:

Entity entity = scene.getEntityManager().getPool().create();

That's a whole lot of stuff to get before you can create an entity and it feels like the design is leaking implementation details a bit if you have to know about entity pools and "managers" to create them. I think things like "pools" and "managers" (if you stick to this name) should be implementation details of the ECS, not something exposed to the client.

But dunno, I have seen some very unimaginative and generic uses of Manager, Handler, Adapter, and names of this sort, but I think this is a reasonably forgivable use case when the thing called "Manager" is actually responsible for "managing" ("controlling?", "handling?") the lifetimes of objects, being responsible for creating and destroying and providing accessing to them individually. That's the most straightforward and intuitive use of "Manager" for me at least.

That said, one general strategy to avoid "Manager" in your name is just take out the "Manager" part and add an -s at the end. :-D For example, let's say you have a ThreadManager and it's responsible for creating threads and providing information about them and accessing them and so forth, but it doesn't pool threads so we can't call it ThreadPool. Well, in that case, you just call it Threads. That's what I do anyway when I'm unimaginative.

I'm kinda reminded of back when the analogical equivalent of "Windows Explorer" was called "File Manager", like so:

enter image description here

And I actually think "File Manager" in this case is more descriptive than "Windows Explorer" even if there's some better name out there that doesn't involve "Manager". So at the very least please don't get too fancy with your names and start naming things like "ComponentExplorer". "ComponentManager" at least gives me a reasonable idea of what that thing does as someone used to working with ECS engines.

1
  • Agreed. If a class is used for typical managerial duties (creating ("hiring"), destroying ("firing"), oversight, and "employee"/higherup interface, then the name Manager is appropriate. The class may have design issues, but the name is spot-on. Aug 11, 2019 at 16:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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