Hello!

I am currently working in an RTS game in C++.

I read in alot of place that calling something "Manager" is generally bad, because it's too generic and say nothing about what the class really does.

However, i have no idea how to call them instead. Currently i have these:

- ImageManager: It load the images, and every class that need a texture can get it from this class. This is because IO operations are REALLY expensives, so i rather read the images once and use all from the memory.

- Building and Unit Manager: These classes have a container, which contains all Building/Unit which are currently alive. You can update them through this class and it's also check if you clicked to one of them.

- EventManager: I didn't wrote it yet, but this supposed to handle Commands and other events. You can basically post any command or event to execute them in the next iteration of event loop.

Any idea what name would fit better what these classes do?

Thank you very much for the answer!

The manager name comes from putting too much global functionality into a single class. Its a good idea to follow the Single Responsibility Principle and have each class only be responsible for one job.

Just to give an example, in extreme case you could split the ImageManager into TextureCache, TextureUploader, Texture, ImageCache, ImageDecoder, Image and reusable FileCache, FileLoader, MemoryBuffer and MemoryAllocators, though most likely this is way too much.

Building and UnitManager seem to have no clear purpose, you could just use some reusable templated container class and add functions in the relevant subsystems doing work on the whole container.

EventManager I would skip, too. A Queue without any special functionality (besides thread safety if needed), where you put in Events, will do for a while.

The OP is somewhat vague, so I write down some possible pitfalls I see:

Your ImageManager may be problematic since it violates the single responsibility principle if implemented as a single class. The single tasks possibly involved here are (a) allocating local storage, (b) reading a resource from mass storage, (c) uploading the resource, (d) implementing a cache strategy, (e) granting access. Okay, all this together means to manage the resource, but the tasks are too different to be implemented in a single class. Instead, if you understand the manager as a facade granting a small API in front of the wrapped functionality, it is IMHO okay to name it "manager" if you wish.

Your BuildingManager and UnitManager are even more problematic. They implement some kind of scene setting but also implement collision detection and update services!? These tasks are not related and IMHO can not even be subsumed in a single "management".

It reads like your EventManager can be named EventProcessor. However, if it executes commands that are generated by events, it is perhaps a CommandProcessor supplied by an EventDispatcher? Because "handle Commands and other events" doesn't seem me to fit in a single class.

EDIT: wintertime was a bit faster than me, and I second those post :)

Your ImageManager may be problematic since it violates the single responsibility principle if implemented as a single class. The single tasks possibly involved here are (a) allocating local storage, (b) reading a resource from mass storage, (c) uploading the resource, (d) implementing a cache strategy, (e) granting access. Okay, all this together means to manage the resource, but the tasks are too different to be implemented in a single class. Instead, if you understand the manager as a facade granting a small API in front of the wrapped functionality, it is IMHO okay to name it "manager" if you wish.

I try to be a bit more accurate, there is not that much functionality in the ImageManager.

It has an Init function, which load the textures to a container, and have a function like that: sf::Texture getTileTexture (Image::TileTexture texture);

Which give back the given texture (TileTexture is an enum)

So it does 2 things: Load the textures, and give an interface to get the texture you need.

Your BuildingManager and UnitManager are even more problematic. They implement some kind of scene setting but also implement collision detection and update services!? These tasks are not related and IMHO can not even be subsumed in a single "management".

No, these classes just iterate through all Units and Buildings, and call the update on them. Everything else is done by the given Unit's or Building's update function.

ImageLoader then, or ImageCache. Or an ImageCache in which you plug an ImageLoader. Caching strategy doesn't depends on the format of the image IMO so you could plug different ImageLoaders for different formats and the ImageCache could store the data loaded by them. Then if you don't want a cache for certain things, just use the loader directly.

ImageCache sounds fine, thanks.

Then that "update" function should do less things.

Well, it updates the objects with the elapsed time. Yes, this can mean alot of things, like train units, move, attack and other stuffs, but i feel like it's reasonable.

I try to be a bit more accurate, there is not that much functionality in the ImageManager.
It has an Init function, which load the textures to a container, and have a function like that: sf::Texture getTileTexture (Image::TileTexture texture);
Which give back the given texture (TileTexture is an enum)
So it does 2 things: Load the textures, and give an interface to get the texture you need.

No, these classes just iterate through all Units and Buildings, and call the update on them. Everything else is done by the given Unit's or Building's update function.

ImageCache sounds fine, thanks.

Well ... notice the process: A client requests the resource library (to avoid the word "manager" here ;)) to return a specific resource. The library requests the cache for the resource. The cache does not store it, so returns nil. The library decides which storage allocator to use, and triggers the loader to load the resource by utilizing the storage allocator. The loader investigates its directory structure (keywords are archive file and file overriding) and detects the file. It looks for a suitable file format, requests an importer from it, and calls it to read the resource. The resource is then returned to the library. The library looks which cache policy is installed for that resource, and puts the resource into the belonging cache. It then returns to the calling client.

From such a description where the single tasks are sectored, one can see how many instances are actually involved in such a "simple" thing like resource management. Many of them can be agnostic of the actual resource type, some can (or should) not. Almost all of them need not know how the resource is encoded on mass storage. Many aspects can be configured (like the storage allocation policy, the cache policy) or are somewhat runtime dependent (like the file format) or are not relevant to the client (like is the resource cached or not). Implementing all this into a single class is wrong, as well as exposing it into the wild is wrong (*), regardless of how the class is named.

You have already mentioned that your process is not that comprehensive, and that you rely on 3rd party libraries. However, the above should demonstrate how a comprehensive solution may look like in detail, especially how many classes are involved in dependence on the inherent tasks.

(*) This is why I would not name the facade class "ImageCache".