Excerpts follow (I've not included items which were specific to that project or clearly overly C-related, although there's still a bit of the latter poking through):
Every bug which is removed during the code/unit test/integration test phases is one less bug which has a chance of finding its way into the finished product and into a customer's workflow. Code reviews allow others to look at a piece of code and to consider if there are logic errors.
We need to determine ways to either not write bugs or make them jump up and announce themselves when they occur. Our goal is to do the fun stuff of writing more, better, code, not the dull part of spending our time finding bugs.
Bugs can be introduced from a variety of sources, including techniques which make it difficult to either track what is happening or to see what is changing. We need to eliminate the stylistic approaches which make this sort of development possible so we increase our chances of not writing buggy code to begin with.
If we have common coding styles, it makes it easier for any developer to pick up a piece of code and have a chance at figuring out what it does. This is especially important for the 2:00 AM phone call about a system problem.
There is almost always a better algorithm or approach to any problem. By conducting a code review, other developers can provide input to find out if there are better (simpler, easier) ways to do anything. If so, let's implement them.
Sometimes we stumble across a truly better way of doing anything. We need to make sure that other developers find out about it so we can spread it through the team and project.
Every program which is written has both a function and a form - the what and how of the code. The customer for a program's function is, indeed, the end-user customer. However, the customer for a program's form is the programmer who must, bleary-eyed, debug a production problem at 2:00 in the morning. Make the form clear to even a novice programmer so even a novice can fix it.
There are very few warnings which are impossible to make go away. For these warnings, understand them and make a comment about it in the code. Otherwise, create type casts or use parentheses or whatever is necessary to make the warnings go away.
Clear code is more important than hand-tuned code. Most modern compilers can out-optimize hand-tuned assembly language. Spend your time productively - writing new, clear functionality, not trying to out-think the compiler. It is more important to make the purpose of code clear than it is to come up with the absolutely, positively most efficient approach (but make sure the purpose is clear).
If you find yourself writing the same exact code more than once, this is a clear indication that one of the following is true:
Variables should either be completely self-documenting (by grace of their name) or should have a comment explaining their use when they are declared.
I also handed out a copy of Steve McConnell's Code Complete to everyone, along with the comment that I didn't agree with everything he said, but I could explain where and why I didn't.
In reply to Re: How do you critique another person's code?
by bmcatt
in thread How do you critique another person's code?
by Rhose
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |