(no title)
dothething | 11 years ago
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.
manifestsilence|11 years ago
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
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
Personally I enjoy reviewing and getting reviewed. In both cases I have learned a lot.
Yacoby|11 years ago
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
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)