Mike Hadlow is a Brighton, UK based developer, blogger and author of a number of open source frameworks and applications. Mike is a DZone MVB and is not an employee of DZone and has posted 88 posts at DZone. You can read more from them at their website. View Full User Profile

What I look for in a Code Review

06.01.2009
| 5924 views |
  • submit to reddit

I recently put this bullet point list together for the team I’m currently working with.

Naming Conventions

General Principles

  • The core imperative is to organise complexity.
  • Clarity and readability is central. “Intention Revealing”
  • Do not prematurely optimise for performance.
  • Do not repeat yourself. Never copy-and-paste code.
  • Decouple.
  • Always try to leave the code you work on in a better state than before you started (the ‘boy scout’ principle)

Keep the source clean

  • Always delete unused code. Including variables and using statements
  • Don’t comment out code, delete it. We have source control to manage change.

Naming things

  • The name should accurately describe what the thing does.
  • Do not use shortenings, only use well understood abbreviations.
  • If the name looks awkward, the code is probably awkward.

Namespaces

  • Namespaces should match the project name + path inside the project. This is what VS will give you by default.
  • Classes that together provide similar functions should be grouped in a single namespace.
  • Avoid namespace dependency cycles.

Variables

  • Use constants where possible. Avoid magic strings.
  • Use readonly where possible
  • Avoid many temporary variables.
  • Never use a single variable for two different puposes.
  • Keep scope as narrow as possible. (declaration close to use)

Methods

  • The name should accurately describe what the method does.
  • It should only do one thing.
  • It should be small (more than 10 lines of code is questionable).
  • The number of parameters should be small.
  • Public methods should validate all parameters.
  • Assert expectations and throw an appropriate error if invalid.
  • Avoid deep nesting of loops and conditionals. (Cyclomatic complexity).

Classes

  • The name should accurately describe what the class does.
  • Classes typically represent data or services, be clear which your class is.
  • Design your object oriented schema deliberately.
  • A class should be small.
  • A class should have one responsibility only.
  • A class should have a clear contract.
  • A class should be decoupled from its dependencies.
  • Favour composition over inheritance.
  • Avoid static classes and methods.
  • Make the class immutable if possible.

Interfaces

  • Rely on interfaces rather than concrete classes wherever possible.
  • An interface is a contract for interaction.
  • An interface should have a single purpose (ISP)

Tests

  • All code should have unit tests if possible.
  • Test code should have the same quality as production code.
  • Write code test-first wherever possible.

Error Handling

  • Only wrap code with a try..catch statement if you are expecting it to throw a specific exception.
  • Unexpected errors should only be handled at process boundaries.
  • Never ‘bury’ exceptions.
References
Published at DZone with permission of Mike Hadlow, author and DZone MVB. (source)

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Perica Milošević replied on Tue, 2009/06/02 - 2:15am

Nice list. Valid for Java, too.

Natasha Van replied on Tue, 2009/06/16 - 1:14am

true

 

Garry Wertu replied on Fri, 2009/10/02 - 4:49am

Excellent list! watch gamer online and watch 500 days of summer

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.