Corey Coogan

Python, .Net, C#, ASP.NET MVC, Architecture and Design

  • Subscribe

  • Archives

  • Blog Stats

    • 110,719 hits
  • Meta

Archive for the ‘SOLID’ Category

Holy Over Mocking Batman: A Natural Progression

Posted by coreycoogan on January 28, 2010


Uncle Bob has been on an interesting kick lately, writing thought provoking posts with some controversial undertones. A few days ago, he posted “Mocking Mocking and Testing Outcomes“, where he mocked the [over?]use of mocking frameworks. It’s an interesting article and worth a read.

I can certainly see where he is coming from with this post. The part that really grabbed me was the last paragraph:

“Also, why do you need to verify that “verify(manager).createCredential” was called? If you really get the credentials you’re expecting from the Authenticator, why would you need to check where they come from? Isn’t it one more way to couple your test to the implementation?”

I think this is the single most common pitfall of anyone who has been doing TDD with mocking for a short period of time.  The initial tendency is to test each and every interaction between the objects.  It usually starts out with large tests that make too many assertions.  It’s not uncommon in these tests to see the developer assert that:

  1. The application service layer gets called
  2. A transaction is started
  3. The appropriate repository method[s] are called, which can be multiple repositories
  4. Some business logic is executed
  5. The right value is returned

Such a test is going to be very brittle and break upon any serious refactoring.  As Uncle Bob points out, do we really care about each of these interactions?  Sure, in some cases it may be important to make sure a transaction is started, but in most cases all you care about is that the right value is returned.  This is the natural progression of implementing TDD, especially when mocking gets introduced.  I think it takes the pains of these brittle tests to discover that asserting all those interactions is really not necessary.

The next progression starts is when the developer begins to understand what one assertion, or logical assertion group, per test really means.  This phase is a little less painful because at least now there is a separate test for each interaction, which is still typically unnecessary.  Now that the tests are broken up into logical units, the tests are easier to fix when refactoring breaks the interactions.

Now the final progression – the pains of these fragile tests lead to the notion that all these tests really don’t matter.  They really don’t prove anything about the quality of the software.  The tests match the implementation and the system depends a set of very brittle tests that could make refactoring too painful.  That’s when the tests get simpler.  That’s when the realization that a mock isn’t needed for everything and ever interaction doesn’t need to be tested.

If you are reading this and it sounds like you are in phase 1 or 2, I highly recommend you read the post “Three simple Rhino Mocks rules” by Jimmy Bogard.  It really influenced the way I write tests.

Advertisements

Posted in Alt.Net, Architecture and Design, Rhino Mocks, SOLID, TDD | Tagged: , , | 2 Comments »

Open/Closed Principle and Over Engineering

Posted by coreycoogan on June 18, 2009


There was a post on the Alt.net group today about how a Factory could violate OCP (Open/Closed Principle) if new methods were added. I’ve had discussions on this issue in the past and figured it deserved a post.

The Open/Closed principle is a software design principle that says:

software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification

I first learned of this principle from one of Robert C. Martin’s books, as it makes up the O in the SOLID principles. I respect SOLID and found that if you do nothing but adhere to these principles, you’ll generally design better software. It’s necessary, however, to use good judgment when applying these guidelines.

In general, OCP is the only SOLID principle that I take issue with, specifically the literal interpretation of “closed for modification”.  It seems that many developers take this to the extreme and spend unnecessary effort to avoid having to ever modify a class or method after it has been written.  The poster in the Alt.net thread was worried that adding an IF block to a method would violate OCP and was asking advice for how to avoid it.

My interpretation of OCP is that the interfaces or API’s should be closed for modification so that any future changes won’t break client or other calling code.  That means keeping your method signatures the same and not changing properties and access modifiers.  Software projects are typically time and resource sensitive and many design decisions must be weighed against the ROI they achieve.  Introducing a more complex design so that a few lines of code don’t have to be added later seems ridiculous to me.

Here’s an example where OCP doesn’t provide the necessary value to make it worth the effort.

Let’s say you have an Order entity and that entity has a Validator class.  The Validator class validates that the Order has all the required values in the right format.  This is how most of us handle validation, using some IValidator<TEntity> type of pattern.  Now let’s say that this Order class goes to production and functions properly for several months.  Now the business comes back and says that Credit Card Security Code is required for PCP compliance.  If we follow OCP in a literal sense, we can’t just add that validation rule to the Validator class.

If a person is designing with OCP in mind, they may use the Specification pattern to handle validation, which would allow us to add validation rules any time without modifying any existing classes.  That is certainly a viable option, but is not as simple and easy to understand as a single Validator class and would certainly be less effort to implement.  I would also argue that the quality of my software with a Validator class is no less than an application using Specifications for validation rules.  If my Validator is using the Notification Pattern to return validation status and error messages to various layers, adding this additional validation rule will require no modification by the rest of the application, which is what OCP means to me.

Conclusion

The SOLID principles and OCP are guidelines to help us deliver more extensible and maintainable software.  However, being a rigid and literal follower of OCP and the “closed for modification” philosophy can lead to over engineering.  Adding complexity for the sake of never touching a class or method again after it’s been written may have its place in some software projects, but chances are in most cases your quality won’t suffer by adding an IF block.

Posted in Architecture and Design, Design Patterns, SOLID | Tagged: , , | Comments Off on Open/Closed Principle and Over Engineering