Which class structure is more desirable?

Question

I'm not sure which of these two “patterns” is the best. I am currently using option A (in conjunction with a provider to implement persistence), but now I am mistaken in the direction of B, especially in light of unit tests that can use the dependency injection model.

Option A:

class ClassA { ClassA() { } Save(); static List<ClassA> GetClassAs(); } 

Option B:

 class ClassA { ClassA() { } Save(); } class ClassARepository { ClassARepository() { } List<ClassA> GetClassAs(); } 

I think I'm asking, is it good practice for a class to expose static methods that return collections of instances of itself?

Edit

There seems to be a general consensus that option B is the best choice. It looks like I have a lot of refactoring ahead: S

Answer 1

Option B is a bit like an ActiveRecord template (Assume that the Save method in ClassA will use ClassARepository?), Which is good in some situations, but if you have a rather complex domain model, I would not use the ActiveREcord template.

Instead, I would use a model like this:

 public class ClassA { public int Id {get; private set;} public string Name {get; set;} } public class ClassARepository { public ClassA Get( int id ); public void Save( ClassA item ); } 

This means that all the logic associated with persistence is placed in the ClassARepository class, and ClassA also does not have direct access to the repository.

Answer 2

You either completely share the constancy of the object with data and logic, or you save everything in one class. Move Save to ClassARepository or save GetClassAs inside ClassA.

Answer 3

Option B looks better because A mixes two ideas, two kinds of functionality. There is a real class A function and a separate collection management function A.

As the application grows, it is also likely that you will need additional functions in your repository - findById, getting subsets, etc. Deflate it into class A will be random.

And, as you already noted, option B is easier to test.

Answer 4

No, I would not consider this bad practice tough and fast, although I assume that it is for some kind of instance tracking or resource monitoring? If so, just remember that there must be logic behind the scenes to ensure that objects that would otherwise be unavailable will not only be inserted into your static collection. I'm not sure which language you use, but if it's C #, you can consider it internally as a List<WeakReference> .

Answer 5

Option B is more flexible.

• This allows you to use multiple repositories.
• It allows a subclass of the repository class.
• It allows you to pass the repository as an argument to another class or function instead of having this class or function explicitly associated with a global static variable.

Answer 6

I think I'm asking, is it good practice for a class to expose static methods that return collections of instances of itself?

As a rule, no - such collections are often single-point, and if they (as in your example), then the usual reasons for single numbers are considered antipattern , as well as the reasons why static is considered harmful .