Is it OK to use manager classes as Singletons?

For each concrete class I have a Manager class. This class has methods like GetAll(), GetById(), Save, etc.

I've made these manager classes a Singleton because I always need one instance, and I have the ability to cache results. For example, when I call GetAll() and the next time I need this method again, the manager don't have to go to the database, it can return the cached results.

Is this a good approach? Or is there a better a开发者_如何转开发lternative way?

What you call manager classes are really "repositories"

Repositories should only work at an aggregate root level, not one repository per class. E.g. if I have an Order class which has a collection of OrderItem, then an Order repository would exist that would have Get/GetAll methods - as the Order is the agg root in this case.

All repository classes would usually be singleton classes, where you can usually enforce this easily via an IOC container.

Overall, I would say your approach of one "repository" per entity is bad - stick to one repository per aggregate root instead.

Why not include them as part of the concrete class but static? Saves the need for two seperate classes.

It sounds like you are close to implementing the Repository pattern, but not quite all the way there. I'd suggest looking into it. I wouldn't make them Singletons -- it makes it too hard to mock them out for your unit tests so you end up adding back doors to defeat the Singleton for testing. A cache makes a nice Singleton object, but why not simply share the cache this way instead of multiplying Singletons?

For testing/mocking purposes, I would advise against using a Manager/Repository Singleton.

If you do want to cache results, then I would suggest delegating to a dedicated Cache class. Any static magic can be contained within the Cache class and your Manager/Repository's semantics can be kept clean.

From a Single Responsibility Principle point of view, I should be able to understand how a Manager/Repository works without having to understand your caching scheme.