Expanding on “Code Reviews Trumps Unit Testing, But They Are Better Together”
Michael Delaney, a senior consulting software engineer at Blue Slate, commented on my previous posting. As I created a reply I realized that I was expanding on my reasoning and it was becoming a bit long. So, here is my reply as a follow-up posting. Also, thank you to Michael for helping me think more about this topic.
I understand the desire to rely on unit testing and its ability to find issues and prevent regressions. For TDD, I’ll need to write separately. Fundamentally I’m a believer in white box testing. Black box approaches, like TDD, seem to be of relatively little value to the overall quality and reliability of the code. Meaning, I’d want to invest more effort in white box testing than in black box testing.
I’m somewhat jaded, being concerned with the code’s security, which to me is strongly correlated with its reliability. That said, I believe that unit testing is much more constrained as compared to formal reviews. I’m not suggesting that unit tests be skipped, rather that we understand that unit tests can catch certain types of flaws and that those types are narrow as compared to what formal reviews can identify.
Here is how I view the constraints around unit testing:
First, unit tests are normally implemented by the same developer who created (or will create) the code. This is a huge constraint on the brain power (BP) being applied to the quality of the code. In this case BP=1. If the unit tests are created by a separate individual then BP>1 and, I argue, BP<2 because the tester is focused on the micro level (the unit level of code).
Second, unit testing is often used to only check that the code does what it is supposed to do. This means unit tests don’t often check whether code does things it isn’t supposed to.
Third, unit tests don’t consider maintainability of the code, or really any macro-level concerns. Passing unit test doesn’t mean that the code is easy to understand or logically organized. I agree that single-purpose methods and well written code are easier to test, but bad code can be tested and even meet high coverage requirements.
The reason I believe that the study’s results (discussed in the previous post) are probably still accurate is that many software fundamentals are no different now than in 1986. We used unit testing frameworks (sometimes called scaffolds) and leveraged unit tests to prevent regressions. In some ways we had to be more concerned with the completeness of our test scenarios in 1986 due to the overhead of modifying an application. Developers today can be somewhat cavalier with code changes since our system environments tend make distribution of updates easier.
I agree that new languages, paradigms and environments have not reduced the need for unit testing, quite the contrary. We have more options and threats to deal with. However, we can more quickly make and distribute an updated application than we could in 1986.
Considering the breadth of what can be identified, formal design and code reviews go beyond simply checking that for some inputs a valid output is calculated. Instead, formal reviews apply BP=T(RE%) where T is the team size and RE% is the review effort percentage the team invests.
These reviews catch maintenance issues, identify best practices violations (beyond what a style checker would do), find opportunities for refactoring (reducing code, reducing complexity, etc.) and serve as a way for the team to increase its overall expertise through interactions and discussion. Clearly, this represents a much broader value set than that provided by unit testing.
As I said in the original posting, this thinking isn’t a defense to skip unit testing in favor of formal reviews. (I clamor for unit tests and high coverage metrics.) Instead, my message is a call to understand that you need both (and other types of testing and inspections), realizing that the formal reviews catch a more comprehensive set of issues than unit testing. Time and again I have seen organizations leveraging unit testing but not formal reviews. This means that their RE% is zero. For me this is a poor investment choice.
Perhaps it would be worth doing a small scale study to see if our shop has a similar experience? It might be quite informative for the whole team.
Tags: efficient coding, Information Systems, linkedin, Managing IS Projects, programming, Testing