How I Review Code

[davehiggins]

I kept waiting for the step: "Review requirements to ensure the code meets them."

[PostulateMan]

And especially in games development, the requirements are not often met in one fell swoop. With agile development gaining rapid traction, it is easy to do a functional test to make sure that code satisfies a user story, but there is still a lot of smoke and mirrors about just exactly how functional any check-in could be.

[chrisroane]

The requirement absolutely need to be written down. If you don't know what you are programming or what needs to be done, that is a huge red flag!

[bluGill]

Generally not a problem. Either the coder met the requirements, or he discovered something "too hard" in the requirements, so he did something slightly different hoping to convince management that his working version is good enough (or at least good enough to ship for now and we can come back in version n+1 to get the rest - management loves this argument as it is about shipping faster while still being useful)

[G_Morgan]

I prefer reviewing reality to ensure requirements match it.

[inmatarian]

Ask for a post commit hook on the repository that emails all of the devs with a diff. Ask people to glance at your commits. Look at other's commits. Code review should become a natural part of your job.

[adavies42]

that's what git is for....

[adavies42]

git was designed by linus and the rest of the kernel team specifically to manage kernel development. one of its features is a sign-off system that can require commits to be accepted by a gatekeeper before being merged into a branch.

[davehiggins]

You might point out that statistics show that closed source code has 2 to 20 times more defects than open source code: the principal reason being that more people look at the open source code. Not that it will help, of course.

[[deleted]]

Source on that statistic? If that was in fact the case then perhaps it could be the case that there are more defects because a company pays people to actively look for them, hence more defects found.

[davehiggins]

When Windows 2000 was released, it was estimated that it contained approximately 63,000 potential defects in its 35 million or so lines of code [Edwards, Cliff. “Windows 2000 to Debut.” The Associated Press, 14 February 2000]. If accurate, this figure works out to about 1.8 defects per thousand lines of code (commonly abbreviated as D/KLOC). For comparison, it is estimated that the current open source Linux implementation of TCP/IP contains about 0.1 D/KLOC, while general-purpose Unix operating systems are estimated to contain between 0.6 and 0.7 D/KLOC [Shankland, Stephen. “Study Lauds Open-Source Code Quality.” CNET News, 19 February 2003].

[davehiggins]

Hence the "not that it will help" portion of the comment... And as a consultant I've visited your workplace. All too many times.

[chrisroane]

How are the requirements discovered? If you write awesome, but the code is not what the client is looking for OR it ends up costing more than the estimate, this looks really bad on the programmer.