SOLID – SOLID Principles vs. Static Methods

object-oriented-designsolid

Here's a problem I frequently run into: Let there be a web shop project that has a Product class. I want to add a feature which allows users to post reviews to a product. So I have a Review class which references a product. Now I need a method that lists all reviews to a product. There's two possibilities:

(A)

public class Product {
  ...
  public Collection<Review> getReviews() {...}
}

(B)

public class Review {
  ...
  static public Collection<Review> forProduct( Product product ) {...}
}

From looking at the code, I'd choose (A): It's not static and it doesn't need a parameter. However, I sense that (A) violates the Single Responsibility Principle (SRP) and the Open-Closed Principle (OCP) whereas (B) doesn't:

  • (SRP) When I want to change the way reviews are collected for a product, I have to change the Product class. But there should be only one reason why to change the Product class. And that's certainly not the reviews. If I pack every feature that has something to do with products in Product, it'll soon be clattered.

  • (OCP) I have to change the Product class to extend it with this feature. I think this violates the 'Closed for change' part of the principle. Before I got the customer's request for implementing the reviews, I considered Product as finished, and "closed" it.

What is more important: following the SOLID principles, or having a simpler interface?

Or am I doing something wrong here altogether?

Result

Wow, thanks for all of your great answers! It's hard to pick one as official answer.

Let me summarize the main arguments from the answers:

  • pro (A): OCP is not a law and readability of the code matters as well.
  • pro (A): the entity relationship should be navigable. Both classes may know about this relationship.
  • pro (A)+(B): do both and delegate in (A) to (B) so Product is less likely to be changed again.
  • pro (C): put finder methods into third class (service) where it's not static.
  • contra (B): impedes mocking in tests.

A few additional things my colleges at work contributed:

  • pro (B): our ORM framework can automatically generate the code for (B).
  • pro (A): for technical reasons of our ORM framework, it will be necessary to change the "closed" entity in some cases, independently from where the finder goes to. So I won't always be able to stick to SOLID, anyway.
  • contra (C): to much fuss 😉

Conclusion

I'm using both (A)+(B) with delegation for my current project. In a service-oriented environment, however, I'll go with (C).

Best Answer

What is more important: following the SOLID principles, or having a simpler interface?

Interface vis-a-vis SOLID

These are not mutually exclusive. The interface should express the nature of your business model ideally in business model terms. The SOLID principles is a Koan for maximizing Object Oriented code maintainability (and I mean "maintainability" in the broadest sense). The former supports the use and manipulation of your business model and the latter optimizes code maintenance.

Open/Closed Principle

"don't touch that!" is too simplistic an interpretation. And assuming we mean "the class" is arbitrary, and not necessarily right. Rather, OCP means that you have designed your code such that modifying it's behavior does not (should not) require you to directly modify existing, working code. Further, not touching the code in the first place is the ideal way to preserve the integrity of existing interfaces; this is a significant corollary of OCP in my view.

Finally I see OCP as an indicator of existing design quality. If I find myself cracking open classes (or methods) too often, and/or without really solid (ha, ha) reasons for doing so then this may be telling me I've got some bad design (and/or I don't know how to code OO).

Give it your best shot, we have a team of doctors standing by

If your requirements analysis tells you that you need to express the Product-Review relationship from both perspectives, then do so.

Therefore, Wolfgang, you may have a good reason for modifying those existing classes. Given the new requirements, if a Review is now a fundamental part of a Product, if every extension of Product needs Review, if doing so makes client code appropriately expressive, then integrate it into the Product.