Should a class contain a collection of itself? [closed]

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 4 years ago.

开发者_StackOverflow Improve this question

Is it poor design to have a class containing a collection of itself like in a List<> for C#? Why is this and what is a better approach and why?

EDIT: More specifically, I have a class called Product and a method in the class called GetProducts() that returns a List collection. It good be grabbing the products from a database, flat file, or xml... not sure yet.

Thanks!

---

With the Product and GetProducts() update I think this might not be such a good thing.

I use a rule of thumb kind of a principle here that relies on the logic that is domain, not language, specific. So in your case I would ask myself: "Does my product contain other actual products?" If the answer is yes then the collection of products is a perfectly legit thing to have in the Product class. If not ask what actually contains these products. A store maybe?

There is an exception to this rule with static methods. If GetProducts() is static then the upper reasoning does not apply and is usually perfectly fine to have it in the Product class.

---

Personally, I'd use the repository pattern:

public class IProductRepository
{
    IEnumerable<Product> GetAll();
}

Then write an implementation:

public class ProductRepository
{
    public IEnumerable<Product> GetAll()
    {
        // Database logic or read from an xml file... etc.
    }
}

Pass an IProductRepository to the caller (using an IoC container like Ninject or Castle Windsor). Then, if necessary, you can easily mock IProductRepository for testing with the caller.

This way you're keeping the actual model (Product) separate from "what you can do" with products.

But, if Products also needed to have Products (example: SubProducts), you could have an ICollection<Product> on Product as well.

---

This is fine. You've come up with a tree or a graph

---

Sure that's generally fine. That's generally the sensible way of implementing a tree in an OOP-y way; a TreeNode has to contain a List<TreeNode> m_Children field in it so the node knows what it's children are for tree traversal.

---

As long as it does not conflict with single responsibility principal I suppose it would not be a problem.

---

It might be both poor and good, depending on the problem you're solving, and if it is a generalized object or not. Many factors may influence such design decisions. In the end, it is no matter of good or bad design, but if this is the right path you're choosing to cross the river.

EDIT #1

OK, I have a class called product and a method inside called GetProducts that returns a List and I wasn't sure if this was the right approach.

Under this circumstance, I would make the Product.GetProducts method static. As such, when you want to load the products, you will simply address your Product class like so:

IList<Product> products = Product.GetProducts();

A list of itself would assume that a product might be composed of different other products, like those component products. But, with a static method, it would more admittedly make your Product class the factory for product related business.