Peer Review

We use Peer Reviews during coding for a variety of reasons. The three main reasons are:

  1. Quality Assurance – checking other developers work to make sure everything is done correctly and works as it should.
  2. Cross Training – the reviewers going over what was done ensures that at least two people have a good understanding of everything that is built and changed. By rotating reviewers for different tasks in the same part of a system many people will be looking at what is built as it is happening.
  3. Education – the developers and peer reviewers learn from each other during the review process – new tricks or techniques are shared and developed.

Peer Review Checklist

We use a detailed checklist that the Peer Reviews go over. Not every item is always applicable, but noting that it is not applicable in that situation is better than forgetting things that are. Our developers are good, but being smart doesn’t mean you can’t forget to check something. Study’s have shown that having clear checklists and procedures to consistently follow helps prevent critical mistakes. Surgeons are encouraged to run through checklists like this one before and during surgery to prevent a simple oversight that could lead to a terrible result. Coders benefit from the same consistent practices to ensure code quality, security, and reliability.

  1. UI
    1. Did we use standard classes for formatting where we have them?
    2. Did we match existing UI patterns (e.g. tables with details)?
    3. Are the labels and instructions clear, understandable, and jargon free?
    4. Do status / error messages show up and do they make sense?
    5. Does the UI work?
    6. Do you escape all displayed user input in the UI (even hidden inputs and data) and emails using encodeForHTML, encodeForHTMLAttribute, etc.?
  2. Code
    1. Is all code object oriented? If not, why?
    2. Are default parameters set? Do they make sense?
    3. Are variables scoped?
    4. Are variable names reasonable and matching our standards?
    5. Are permissions / login checked?
      1. Are remote functions properly checked for logins and permissions?
    6. Has the execution speed been checked? Is it fast enough?
      1. Have any needed database indexes been added?
      2. Are there any inefficient queries or loops?
    7. Have you done a global search for any functions that have been altered in any way to ensure that it didn’t break something elsewhere – e.g. changing a cfargument that is used when the function is called somewhere else could cause a query to return too many or wrong results and lead to a data breach
  3. Data
    1. Are all inputs validated for completeness and format (e.g. valid dates or emails)?
    2. Are required fields clearly marked and validated for?
    3. Does the data save correctly?
    4. Do we store enough meta data (e.g. updater_ID, date_updated, etc.)
    5. Do we log anything we need to for HIPAA or other use?
Kevin Hall
Latest posts by Kevin Hall (see all)