Sign in to follow this  

Question about Managers

This topic is 2265 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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 [i]why [/i]you shouldn't do it, and in what exceptional cases you [i]can [/i]use that solution.

Share this post


Link to post
Share on other sites
[quote name='Aardvajk' timestamp='1319046045' post='4874388']
When a class name becomes vague, the temptation to blur its responsibilities increases.
[/quote]

[code]class ManagerManagerManager
{
LightManagerManager LightManager;
ObjectManagerManager ObjectManager;
};[/code]

Share this post


Link to post
Share on other sites
As mentioned above, manager classes are usually bad because they have very vague responsibilities. In a good design, each class should have [url="http://en.wikipedia.org/wiki/Single_responsibility_principle"]only one[/url] 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.[quote name='Akusei' timestamp='1319041885' post='4874367']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?[/quote][s]class StateManager;[/s] [font="Courier New"]class StateMachine {[/font]
private:
[font="Courier New"] StateContainer m_container;//responsible for init/cleanup[/font]
[font="Courier New"] StateQueue m_queue;//responsible for queueing[/font]
[font="Courier New"]};[/font]
[quote]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[/quote][s]class AssetManager;[/s] [font="Courier New"]class AssetRepository;[/font]
Make those changes and you're no longer using "manager" classes [img]http://public.gamedev.net/public/style_emoticons/default/cool.gif[/img]

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
[quote name='way2lazy2care' timestamp='1319222727' post='4875116']
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.
[/quote]

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.

Share this post


Link to post
Share on other sites
[quote name='way2lazy2care' timestamp='1319222727' post='4875116']out of curiosity how do you guys feel about manager classes in data oriented design rather than object oriented design?[/quote]There's actually a great example of this in most game engines ([i]even heavily OOD ones[/i]) -- particle systems.
Usually it's realised that treating every single particle ([i]when there may be millions of them[/i]) 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.

Share this post


Link to post
Share on other sites
[font="arial, verdana, tahoma, sans-serif"][size="2"]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.
[code]
// 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
};
[/code]
[code]

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);
}
}
[/code][/size][/font]

Share this post


Link to post
Share on other sites
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 [i]only[/i] 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 [i]think[/i] 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.

Share this post


Link to post
Share on other sites
[quote name='jonbonazza' timestamp='1319319923' post='4875449']...[/quote]You're thinking of singletons, not managers ([i]although in bad designs, it's common for managers to also be singletons[/i]).

Share this post


Link to post
Share on other sites
[quote name='jonbonazza' timestamp='1319319923' post='4875449']
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 [i]only[/i] 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 [i]think[/i] 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.
[/quote]


[quote name='Hodgman' timestamp='1319448569' post='4876252']
[quote name='jonbonazza' timestamp='1319319923' post='4875449']...[/quote]You're thinking of singletons, not managers ([i]although in bad designs, it's common for managers to also be singletons[/i]).
[/quote]
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

Share this post


Link to post
Share on other sites
[quote name='Hodgman' timestamp='1319448569' post='4876252']
[quote name='jonbonazza' timestamp='1319319923' post='4875449']...[/quote]You're thinking of singletons, not managers ([i]although in bad designs, it's common for managers to also be singletons[/i]).
[/quote]

Then what exactly is a [i]Manager[/i]? 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.

Share this post


Link to post
Share on other sites
[quote name='jonbonazza' timestamp='1319487933' post='4876457']
Then what exactly is a [i]Manager[/i]?
[/quote]

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

Share this post


Link to post
Share on other sites
[quote]
Then what exactly is a Manager?
[/quote]
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.

Share this post


Link to post
Share on other sites
[quote name='rip-off' timestamp='1319491015' post='4876482']
[quote]
Then what exactly is a Manager?
[/quote]
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.
[/quote]

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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
[quote name='freeworld' timestamp='1319492046' post='4876485']
[quote name='rip-off' timestamp='1319491015' post='4876482']
[quote]
Then what exactly is a Manager?
[/quote]
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.
[/quote]

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.
[/quote]

Particle manager? lol

Share this post


Link to post
Share on other sites
[quote name='jonbonazza' timestamp='1319487933' post='4876457']Then what exactly is a [i]Manager[/i]? 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.[/quote]There's absolutely no need to make your asset manager or scene manager into global variables.

A class that has a single global instance is a [url="http://c2.com/cgi/wiki?SingletonPattern"]singleton[/url]. "Manager" is just a vague verb that's commonly used to describe classes that are collections of other objects ([i]e.g. an asset manager "manages" a collection of assets[/i]).
As has been said in the thread already, [url="http://c2.com/cgi/wiki?DontNameClassesObjectManagerHandlerOrData"]using vague verbs is discouraged[/url].

Share this post


Link to post
Share on other sites
[quote name='jonbonazza' timestamp='1319487933' post='4876457']
Then what exactly is a [i]Manager[/i]? 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.
[/quote]
How about:
AssetBuilder builds assets.
AssetCollection is a collection of assets of type T.
AssetRegistry has collections of assets for varying types.

None of them need to be global. There is the additional benefit when you pass them around that the public interface for the caller is simpler.

Share this post


Link to post
Share on other sites
Hmm, I find this thread quite interesting as I am making a class on my own which loads sprite textures.

My class has a std::vector<SpriteData>, where SpriteData is a struct with an "std::string name", "int NumberOfUsers" and "LPDIRECT3DTEXTURE9 Sprite".

The purpose of the class is to make sure I don't fill memory with duplicate textures. Anytime I want to load a Sprite Texture I call the function member "LoadTexture(name)". If it finds that it is already loaded by checking for existance in the vector it returns the loaded LPDIRECT3DTEXTURE9 which shares the same std::string name and it increments NumberOfUsers, otherwise it loades the file in to memory, adds the new SpriteData on the vector. And then returns the LPDIRECT3DTEXTURE9.

Is this discouraged?

Share this post


Link to post
Share on other sites
[quote name='Tispe' timestamp='1319544244' post='4876707']My class has a std::vector<Data>, where Data is a struct with an "std::string name", "int NumberOfUsers" and "T* resource".

The purpose of the class is to make sure I don't fill memory with duplicate resources. Anytime I want to load a resource I call the function member "Load(name)". If it finds that it is already loaded by checking for existance in the vector it returns the loaded T* which shares the same std::string name and it increments NumberOfUsers, otherwise it loades the file in to memory, adds the new Data on the vector. And then returns the T*.[/quote]^^I've edited your quote. If you changed your class to be a template class that stored [font="Courier New"]T*[/font]'s instead of [font="Courier New"]IDirect3DTexture9*[/font]'s, then it would be a reusable [i]reference-counted-resource-cache[/i], instead of being tightly coupled with a Direct3D-specific type.

[edit - added example] e.g. note that this only has [url="http://en.wikipedia.org/wiki/Single_responsibility_principle"]one responsibility[/url] - caching pointers that are matched to names via reference counting. It's not an "all-in-one manager" that is also responsible for creating/destroying the resources, updating them, etc... which constrains it to a nicely manageable 40 lines of code.[source lang=cpp]template<class T, class Fn> class ResourceCache {
public:
ResourceCache( const Fn& loader ) : m_loader(loader) {}
T* Load( const std::string& name )
{
DataMap::iterator i = m_resources.find(name);
T* result;
if( i == m_resources.end() )
{
result = m_loader.Load(name);
Data data = { 1, result };
m_resources.insert( std::make_pair(name, data) );
}
else
{
i->second.count++;
result = i->second.data;
}
return result;
}
void Unload( T* data )
{
for( DataMap::iterator i = m_resources.begin(), end=m_resources.end(); i != end; ++i )
{
if( i->second.data == data )
{
if( --i->second.count == 0 )
{
m_loader.Unload( data );
m_resources.erase( i );
}
break;
}
}
}
private:
struct Data { int count; T* data; };
typedef std::map<std::string, Data> DataMap;
DataMap m_resources;
Fn m_loader;
};

struct IntLoader {
int* Load( const std::string& name ) {
return new int;
}
void Unload( int* p ) {
delete p;
}
};

void test()
{
IntLoader myLoader;
ResourceCache<int, IntLoader> intCache( myLoader );
int* res1 = intCache.Load("asdf");
int* res2 = intCache.Load("asdf");
intCache.Unload( res1 );
intCache.Unload( res2 );
}[/source]

Share this post


Link to post
Share on other sites
Hodgman, what about something as heightmaps? Let's say I have HeightmapManager, which handles loading Heighmap when it becomes visible, unloading when it wasn't visible for awhile, rendering them, etc?

Share this post


Link to post
Share on other sites
Sign in to follow this