A few days ago, a fellow coworker asked me to help her in fixing some "legacy unit tests". I'll explain why I call them "legacy", eventhough the code they test is relatively new.
The first test method "TestMethodX" tested MethodX that is located on a Presenter class (we use the
Model-view-presenter pattern). We ran it and failed. Ok, lets try to fix it:
1. Following good design principles on the ModelViewPresenter pattern, your method should have just
one responsibility so it has only one reason to change. TestMethodX was long, had a lot of data initialization and used Mock classes in a very confusing way. There were lots of repeated statements and wasn't self documented (or had any code comments whatsoever).
2. On an Object Programming style, you should assign responsabilities to different objects instead of having a large procedural method. The way, your design is loosely coupled and highly cohesive. That's a good thing. But MethodX was very long. It used the View class on strange ways and had lots of ifs, whiles and calls to other private methods. When you try to test such a method, I assure you you'll have a hard time understanding it. It's not fun.
Long methods are evil on several grounds:
- They're hard to understand
- They're hard to change
- They're hard to reuse
- They're hard to test
- They have low cohesion
- They may have high coupling
- They ten to be overly complex
Often programmers ask, how "long" is a long method? A method is too long if
- you have to scroll down to look at the complete method
- it takes several minutes to understand
- you can't easily write an automated test for it
- you can't state the one prominent purpose for the method
3. The method wasn't developed on a TDD style for sure. TDD leads to simple design and promotes clear cohesive code. So, the programmer that did the tests, obviuously did them afterwards and just as a formalism. The final result was some test code that didn't add any value to the development process and to the product itself. Honestly, the original developer wasted his time doing that tests.
After reading the unit test and the tested code, we had two options:
1. To make the unit test pass and have it constantly verifying a confusing designed class.
2. To delete the unit test and just face the truth that the tested method should be redesigned.
At the end, we decided to delete the unit test cause its existence added no value at all. I'd really like to undeline the concept that Unit tests are a strong resource for helping in your design. The enfasis shouldn't be on Test, but on Design. They are not just a formalism, they are useful and the final result is great code. That's what we are talking about.
So, if you are a developer, please give it a try to TDD, write code in responde to a unit test. Test first, test early. Make small methods and write human readable code. We, the rest of the world, would really apreciate it! :-)
