39 replies to this topic

[Akusei]

I've read a few times on this forum that the use of any kind of Manager is frowned upon; in fact, I think I even read somewhere here that if you feel the need to create a class with the word Manager in the name then you're design is wrong.

In the case of a state machine, to me anyway, it kind of makes sense to have a state manager class to handle all the different states and their initialization, queuing, cleanup, etc. Am I wrong in this, or would this be an appropriate place to have a manager?

Oh, and in my game I'm creating right now I've made an asset manager, or resource manager class that handles loading resources from a custom file format into memory and makes for easy retrieval of the needed resource via a simple function call and ID string; is this bad design as well?

Thanks

[Serapth]

Manager's very much have their time and place and if anyone blanket tells you "never use X or Y" generally you should disregard their opinion, unless of course they said to never us DevC++, because you should never use DevC++.


Managers can be quite useful, the pitfall is quite often they are just globals by another name. Then again, sometimes globals can be quite useful too and many times you will hear people say "NEVER USE GLOBALS!", which is rather idiotic, but has a kernel of truth to it. Lets go with "Never use globals, unless they are the best option". The same can be said of Manager classes. Some times it does make sense to have a global manager class of some shape or form, such as your described state machine, you just have to be careful you aren't just creating managers out of lazy design... because they are (initially) easy.


You just have to be extremely careful. On of the issues with using global or manager classes is they touch so many areas of your code, that finding a bug or making a change can be an absolute nightmare. In this regard, make sure ( if you can help it ) that your manager is basically a black box. If any class can effect changes on your manager, you can create an absolute friggin nightmare if you need to bug hunt later. In this case, do your best to make any global/manager classes as tight as possible with the bare minimum number of externally accessible methods. If there is a need to make changes to a manager class, doit through a very narrowly defined interface. Also be sure that the manager always owns everything it requires. Don't ever let an external process destroy or alter something the manager owns, without the manager being explicitly aware of it, preferably by only returning const values. Finally be very careful in regards to thread safety.

[Telastyn]

In addition to Serapth's answer:

If you're a beginner, then I would say do what you think is a good way to solve the problem. There's a difference between reading that you shouldn't do something and doing it to see where things go wrong so you can actually understand why you shouldn't do it, and in what exceptional cases you can use that solution.

[Aardvajk]

I think the reason for the advice you have heard is less to do with globals (this is a valid but different issue) and more to do with the fact that responsibilities of a Manager class tend to grow due to the vagueness of the term.

If a class loads and stores resources, say, this is probably a good candidate for splitting into two, each with its own clear responsibilities. This makes for a system that is cleaner to extend with new types of loader for example.

When a class name becomes vague, the temptation to blur its responsibilities increases.

[yewbie]

When a class name becomes vague, the temptation to blur its responsibilities increases.

class ManagerManagerManager
{
	LightManagerManager LightManager;
	ObjectManagerManager ObjectManager;
};

[Hodgman]

As mentioned above, manager classes are usually bad because they have very vague responsibilities. In a good design, each class should have only one well defined responsibility. Once a class starts taking on multiply responsibilities, it should be split into multiple classes and then composed together.
When a class gets "manager" in it's name, it's usually a hint that it has a lot of responsibilities -- this isn't always the case, but it's true often enough that now "manager" classes are usually treated with suspicion.

In the case of a state machine, to me anyway, it kind of makes sense to have a state manager class to handle all the different states and their initialization, queuing, cleanup, etc. Am I wrong in this, or would this be an appropriate place to have a manager?

class StateManager; class StateMachine {
private:
StateContainer m_container;//responsible for init/cleanup
StateQueue m_queue;//responsible for queueing
};

Oh, and in my game I'm creating right now I've made an asset manager, or resource manager class that handles loading resources from a custom file format into memory and makes for easy retrieval of the needed resource via a simple function call and ID string

class AssetManager; class AssetRepository;
Make those changes and you're no longer using "manager" classes

[Akusei]

Thanks everyone for the input, as usual it was very helpful!

[way2lazy2care]

out of curiosity how do you guys feel about manager classes in data oriented design rather than object oriented design? A better name might be calling it an interface with the data, but as interface already has a pretty well defined definition in most programming, you can't really call it that.

[Serapth]

out of curiosity how do you guys feel about manager classes in data oriented design rather than object oriented design? A better name might be calling it an interface with the data, but as interface already has a pretty well defined definition in most programming, you can't really call it that.

If you work(ed) in corporate IT in n-tier solutions, this is very much the norm. The name is generally the DAL ( Data access layer ), which is essentially a thing abstraction above the application data. In a typical 3 tiered approach you have your view or presentation layer, your business logic layer ( BLL ) and your DAL. The DAL is an abstraction of the data, the view is your UI and then your BLL is your logic.

It actually applies fairly well to gaming too, especially if you want your tools and game to share the same code.

[Hodgman]

out of curiosity how do you guys feel about manager classes in data oriented design rather than object oriented design?

There's actually a great example of this in most game engines (even heavily OOD ones) -- particle systems.
Usually it's realised that treating every single particle (when there may be millions of them) as an independent object isn't the best approach, and instead they're managed as a "system" of particles instead.

I'd still avoid calling it a 'manager' if you can, to properly define the responsibility of this 'grouping' class.

[adam4813]

Well since we are discussing uses for managers as in when they are good. How about a manager that loads, starts, shuts down, and unloads dynamic libraries containing separate systems (graphics, network, etc)?

My code (ignore the name as core does have meaning inside the design for the whole program. I have removed includes as they are irrelevant to the discussion of managers.

// Preprocessor selection based on OS
#ifdef _WIN32
#include <Windows.h>
typedef HMODULE DLLHANDLE;
#else
#include <dlfcn.h>
typedef void* DLLHANDLE;
#endif

class CoreManager {
public:
	CoreManager(GlobalProperties* gprops, MessageDispatcher* msgdisp);


	void Startup(std::string name = "");
	void Shutdown(std::string name = "");
	void Load(std::string name);
	void Unload(std::string name = "");

	void Update(double dt = 0.0f);
private:
	GlobalProperties* gprops;
	MessageDispatcher* msgdisp;

	std::map<std::string, CoreInterface*> coremap; // A mapping of each core to its given name
	std::map<std::string, DLLHANDLE> libraries; // A mapping of each loaded library to a given filename
};

CoreManager::CoreManager( GlobalProperties* gprops, MessageDispatcher* msgdisp ) : gprops(gprops), msgdisp(msgdisp)
{

}

void CoreManager::Startup(std::string name /*= ""*/ )
{
	if (name == "") {
		// We need to use the regular for loop as we may modify the iterators if a core fails to start
		for (auto it = this->coremap.begin(); it != this->coremap.end(); ++it) {
			if (!(*it).second->Startup()) {
				//EventLogger::printToFile( 5,"Core failed to start removing.", 'C'); // TODO: Fix eventlogger
				(*it).second->Shutdown();
				delete (*it).second;
				this->coremap.erase(it++);
				if (it == this->coremap.end()) {
					break;
				}
			}
		}
		this->msgdisp->SetCores(&this->coremap);
	} else {
		if (this->coremap.find(name) != this->coremap.end()) {
			this->coremap[name]->Startup();
		}
	}
}

void CoreManager::Shutdown( std::string name /*= ""*/ )
{
	if (name == "") {
		// We need to use the regular for loop as we may modify the iterators if a core fails to start
		for (auto it = this->coremap.begin(); it != this->coremap.end(); ++it) {
			(*it).second->Shutdown();
			delete (*it).second;
			this->coremap.erase(it++);
		}
	} else {
		if (this->coremap.find(name) != this->coremap.end()) {
			this->coremap[name]->Shutdown();
			delete this->coremap[name];
			this->coremap.erase(name);
		}
	}
}

void CoreManager::Load( std::string name )
{
	if (this->libraries.find(name) == this->libraries.end())
	{
#ifdef _WIN32
		wchar_t buf[256];
		mbstowcs(buf, name.c_str(), name.length());
		HMODULE libdll = LoadLibrary(buf);

		this->libraries[name] = libdll;
#else
		void * libdll = dlopen(fname.c_str(), RTLD_LAZY);
		this->libraries[name] = libdll;
#endif
	}

	CoreFactory fact;
#ifdef _WIN32
	fact = (CoreFactory)GetProcAddress(this->libraries[name], "CoreFactory");
#else
	fact = (CoreFactory)dlsym(this->libraries[name], "CoreFactory");
#endif

	CoreInterface* core = fact(this->gprops, this->msgdisp->GetCallback());
	this->coremap[name] = core;
}

void CoreManager::Unload( std::string name /*= ""*/ )
{

	bool ret = false;

	if (this->libraries.find(name) != this->libraries.end())
	{
		Shutdown(name);
#ifdef _WIN32
		if (FreeLibrary(this->libraries[name]) != 0)
		{
			ret = true;
		}
#else
		ret = dlclose(this->libraries[name]);
#endif
		if (ret)
		{
			this->libraries.erase(this->libraries.find(name));
		}
	}
}

void CoreManager::Update( double dt /*= 0.0f*/ )
{
	for (auto it = this->coremap.begin(); it != this->coremap.end(); ++it) {
		(*it).second->Update(dt);
	}
}

[jonbonazza]

From what I understand (and I could be entirely incorrect on this understanding), managers themselves are not a bad thing, but it's when they are misused that code quality suffers.

Due to their global nature, managers should only be used when a design calls for a class that can only have one existing instance without undefined behavior arising. Coming to such a conclusing is sometimes not as trivial as one might think. Often times, a designer may think that a class should only exist once, while in reality, they are just over-looking a potential application for multiple instances. This is where managers get their bad name from.

The same argument can be applied to global variables in general.

[Hodgman]

...

You're thinking of singletons, not managers (although in bad designs, it's common for managers to also be singletons).

[way2lazy2care]

From what I understand (and I could be entirely incorrect on this understanding), managers themselves are not a bad thing, but it's when they are misused that code quality suffers.

Due to their global nature, managers should only be used when a design calls for a class that can only have one existing instance without undefined behavior arising. Coming to such a conclusing is sometimes not as trivial as one might think. Often times, a designer may think that a class should only exist once, while in reality, they are just over-looking a potential application for multiple instances. This is where managers get their bad name from.

The same argument can be applied to global variables in general.

...

You're thinking of singletons, not managers (although in bad designs, it's common for managers to also be singletons).

This. Managers don't have to be global, and they shouldn't be global. If you make your manager global it is probably the kind of manager that makes other "manager" classes look bad.

http://stackoverflow.com/questions/86654/whats-wrong-with-singleton

[jonbonazza]

...

You're thinking of singletons, not managers (although in bad designs, it's common for managers to also be singletons).

Then what exactly is a Manager? I was under the impression that a manager was a global instance of a class that handles the functionality of a certain aspect of a software. For example, an AssetManager would handle the loading of assets, while a SceneManager would handle the transition of scenes, and so on.

[yewbie]

Then what exactly is a Manager?

Something that detracts from your projects goals and make them harder to understand.

[rip-off]

Then what exactly is a Manager?

That is the problem with managers. They have no clear definition. They often end up as dumping grounds for unrelated functionality. This introduces complexity and coupling that would have been avoided if the class was given a clearer name.

[freeworld]

Then what exactly is a Manager?

That is the problem with managers. They have no clear definition. They often end up as dumping grounds for unrelated functionality. This introduces complexity and coupling that would have been avoided if the class was given a clearer name.

my particle system is technically a manager/factory, though I don't call it that. My manager cleary handles a set of particles. Just because it's called a manager doesn't make it complex. Let alone the name has nothing to do with coupling. Also what sounds more accurate to you Particle System" or "Particle Manager".... what exactly is your particle system doing? Managing particles... right?

Alot of your guys posts are quite wishy washy on this subject. Also my particle system is not a singleton, despite everyone here saying managers have to be singletons. I can create as many instances as ram and processing power will permit without an issue.

[rip-off]

Particle systems are a well understood design. Programmers are less likely to add unrelated functionality to such a class. When I have implemented such classes, I have called them ParticleSystems, not a ParticleManagers. This terminology pre-dates my opinion on the nebulous "manager" phrase.

[return0]

Then what exactly is a Manager?

That is the problem with managers. They have no clear definition. They often end up as dumping grounds for unrelated functionality. This introduces complexity and coupling that would have been avoided if the class was given a clearer name.

my particle system is technically a manager/factory, though I don't call it that. My manager cleary handles a set of particles. Just because it's called a manager doesn't make it complex. Let alone the name has nothing to do with coupling. Also what sounds more accurate to you Particle System" or "Particle Manager".... what exactly is your particle system doing? Managing particles... right?

Alot of your guys posts are quite wishy washy on this subject. Also my particle system is not a singleton, despite everyone here saying managers have to be singletons. I can create as many instances as ram and processing power will permit without an issue.

Particle manager? lol