3 ms·

To me, code review is a combination of two things I would think we would try to avoid: 1. Testing implementation over behavior 2. Using a human being to test in

To me, code review is a combination of two things I would think we would try to avoid: 1. Testing implementation over behavior 2. Using a human being to test in a non-automated way Proper test coverage and human QA to me is the most effective approach to not shipping bugs, because it really doesn't matter to the customer what the code does or doesn't look like if it runs. And I just don't believe that code review is an efficient means to catch that if user is in a state of X and tries to do Y to Z that the app blows up. That becomes most apparent when that code is running in the full context of the application, not when viewed as a git diff. A bug is so often a perfect storm of seemingly innocuous but conflicting/insufficient behavior than it is some block of code that is clearly broken at a glance. There is something to be said about stylistic and architectural issues and code reviews do give an opportunity for asking questions and driving "better" code. But this is actually just another reason why I don't like code review. "Better" is subjective and there's always another level of "better" to be reached. If I held them to my own personal standards sometimes I'd be asking for a rewrite. So what do I do, suggest that they improve their code, but not to the full extent that I feel it could be improved? I hate code review.

eridius 8y ago
There are so many bugs that I've caught in code review that would not have been caught by automated or manual testing. Two big reasons for that: 1. Automated testing is still written by a human. A human who doesn't understand the edge cases in their code isn't going to understand how to write tests for those edge cases either. And "have someone else write the tests" won't work either, because frequently these edge cases are a result of the code implementation, not inherent in the problem, and so without a thorough understanding of the code you wouldn't even know the edge cases is there to be tested. Same goes for manual testing too, the manual testcases are still constructed by a human. 2. Race conditions, or rare conditions that most people won't hit. These are rather unlikely to be uncovered by testing (whether automated or manual). Race conditions in general are hard to test, and rare conditions are, well, rare. There's also just cases like unexpected interaction paths that can produce improperly-handled states in the code, or bugs that don't result in an obvious bug to the observer but are still incorrect (the obvious example here is memory leaks, but this could also just be leaving the app in a bad state such that subsequent actions would fail, but you don't catch those because your test is done at this point).

eridius 8y ago

There are so many bugs that I've caught in code review that would not have been caught by automated or manual testing. Two big reasons for that: 1. Automated testing is still written by a human. A human who doesn't understand the edge cases in their code isn't going to understand how to write tests for those edge cases either. And "have someone else write the tests" won't work either, because frequently these edge cases are a result of the code implementation, not inherent in the problem, and so without a thorough understanding of the code you wouldn't even know the edge cases is there to be tested. Same goes for manual testing too, the manual testcases are still constructed by a human. 2. Race conditions, or rare conditions that most people won't hit. These are rather unlikely to be uncovered by testing (whether automated or manual). Race conditions in general are hard to test, and rare conditions are, well, rare. There's also just cases like unexpected interaction paths that can produce improperly-handled states in the code, or bugs that don't result in an obvious bug to the observer but are still incorrect (the obvious example here is memory leaks, but this could also just be leaving the app in a bad state such that subsequent actions would fail, but you don't catch those because your test is done at this point).