top | item 8862452

(no title)

dothething | 11 years ago

Code reviewing is such a hilarious concept in programming. Everyone thinks it's the most important thing, yet I've only seen rubber stamps and uninterested reviewers.

I'd be so much happier if we could get rid of the entire notion and do up front design and pair programming.

Also, oh my god that checklist. Anyone that has a business dedicated to delivery is going to have to throw that thing out the window.

discuss

order

manifestsilence|11 years ago

I don't know if newer things have contradicted the advice of the Mythical Man Month, but I seem to recall it claims testing finds ~50% of bugs whereas review finds ~80%, with a key caveat being that the two methods tend to find different bugs so the overlap of using both is useful.

I think it also depends on the type of code and the consequences of failure. I have seen good success reviewing data migration and modification code for medical records because that stuff was considered important data and there was a culture there such that if something went wrong, they would ask, "why wasn't this caught in review?", so the process was taken seriously.

I agree with the pair programming sentiment though - I think it could be looked at as a form of code review with a fresher context since you see mistakes as they are made rather than puzzling out the thought process of the other coder when it's cold.

RogerL|11 years ago

I've never been a big fan of code review for finding bugs. Sure, you can find bugs, but the question is, how efficiently?

Long ago I learned a couple simple rules that get my defect rate very close to zero: asserts everywhere, and step through every line of code. Seriously. Write two lines of code, fire up the debugger, and step through them, inspecting each variable. Any tiny 'unit' that you write should go through the debugger, where unit might be that 5 line function you are writing, or a bug fix, or whatever. It just catches a vast amount of the bugs if you are writing in a modular, perhaps functional style. Tons of side effects, well, it's less good at that. asserts do 'okay' at finding those things, but of course the time to find those bugs is longer.

This buys you a few things. First, you are 'reviewing' your code while you write, which is when it is freshest in your mind. It is hard to mentally model the behavior of the computer, hence code reviews are mentally difficult. The debugger ALWAYS shows you what the computer will do (bar things like nasty thread interactions). The debugger does not lie. You get to see nasty things like Nan or underflow that might just pass silently depending on your error handling, which is again hard to find in a code review. And, of course, the sooner you find bugs the cheaper it is to fix - so the best time is about 10 seconds after you typed that line of code. After that the costs grow either proportionally or exponentially.

I know very few people with that uses this discipline, but it changed my life when I took it up. Any time I slip and fail to do it, I rue it.

Pair programming get you similar benefits. I find it harder to think/talk/respond to another person when I am deep in thought (different brain pathways, or so it feels), so I'm more a solo programmer by nature. However, I would still step through the code when pair programming; I guarantee that your mental model of your code is likely to be flawed. The second person will reduce, but not eliminate that threat.

I understand not all code bases/IDEs/whatever allow this (long startups, 5+ minute compiles, etc), but it it is available, avail yourself of it. If you program in a dynamic language and find yourself using a REPL all the time to test your code you are doing a form of this, but it is still better to test the actual code in place.

matwood|11 years ago

I have found code reviews to be a great thing. Obviously not every piece of code can be reviewed, but knowing that someone may look at your code leads people to think about it just a bit more.

Personally I enjoy reviewing and getting reviewed. In both cases I have learned a lot.

Yacoby|11 years ago

I think you need either code reviews or pair programming to ensure quality and bring developers up to a more even standard.

However I am not as fond of pair programming for ensuring quality of a change. I find that often it is easy when pair programming to go down the route that looks good at the time and it is hard to step back and evaluate the whole change. Where as being able to look at the end result without any intermediate steps often makes it easier to find flaws.

I think that with code reviews, I find them easier to do if I view the coder as an adversary. They have produced flaws and I need to find them.

qznc|11 years ago

> Everyone thinks it's the most important thing

I think code review might be the next big thing programmer culture adopts as abolutely necessary. Like we are currently adopting continuous integration and have been adopting version control. This means version control is even more important but we have consensus so there is little to discuss. (just some central vs distributed tradeoffs)