Code Reviews: Boost Quality and Supercharge Your Team

While reviews can add steps to your process, they can ensure a better solution for your users and ultimately higher user adoption. Here, we’ll take a look at some of those benefits and some of the review opportunities available when building solutions on Salesforce.

BENEFITS OF REVIEWS
Benefit #1: Improved Quality
As a Salesforce consultant, your primary goal should be creating robust, correct, usable solutions that are scalable, maintainable, and add value for customers. Reviews are one of the most effective tools when working toward that goal. Logically, applying multiple perspectives to a problem results in solutions that are more likely to be correct. Here, we’ll focus primarily on code reviews, but reviewing architecture, design, and test plans can help uncover “big picture” issues that may be overlooked when focusing on the code. Be sure to consider each of those opportunities when implementing reviews on your project.

In many disciplines, we realize that to err is human and plan accordingly. Consider the number of rewrites, reviews, and edits an author will go through before publishing his final work. As Salesforce consultants, we must realize that we are “authors” and the quality of our work will improve as we seek the input and ideas of our coworkers.
“In a software-maintenance organization, 55 percent of one-line maintenance changes were in error before code reviews were introduced. After reviews were introduced, only 2 percent of the changes were in error. When all changes were considered, 95 percent were correct the first time after reviews were introduced. Before reviews were introduced, under 20 percent were correct the first time.”
~ Steve McConnell, “Code Complete”, p. 481

Benefit #2: Skill Building
While improving the quality of our work, reviews will also spread domain knowledge and improve consulting skills. A brief review of a requirements document or data flow diagram can help communicate details that might otherwise be tough to uncover. As a developer, I can’t count the number of tips and tricks I’ve learned by reviewing others’ code. Code reviews help build productive and skillful programmers.

Benefit #3: Communication
Last but not least, reviews are a very effective way to communicate domain and solution information among team members who may be each working on a different part of an app. Reviewing work products such as source code, workflow rules, or test scripts helps to communicate specifics at the more detailed level. Team mates who analyze each other’s work will understand the comprehensive picture of the app better and produce a more consistent product.

REVIEW METHODS
There are several review methods. Find one that fits the way your team works and implement it!

Formal (Fagan)
This is a careful and detailed process with multiple participants and multiple phases. Formal reviews are the traditional method, in which co workers attend a series of meetings and review line by line, usually using printed copies of the material. Formal inspections are extremely thorough and have been proven effective at finding defects, but also take significant time.
Over the shoulder
In this method, one person looks over the author’s shoulder as the latter walks through the code.
Email pass around (+ Google Docs)
Here, the author or source code management system emails code to reviewers and requests comments, such as through Google docs.
Pair programming (online tools)
“Pair programming” was introduced in the “Extreme Programming” methodology, and refers to two authors working together.
Tool assisted
This refers to any process where specialized tools are used in all aspects of the review: collecting files, transmitting and displaying files, commentary, and defects among all participants, collecting metrics, and giving product managers and administrators some control over the workflow.

The “Tool Assisted” method works well on Salesforce projects and fits in well with other best practices such as revision control and project management systems. When implemented in a Salesforce team, the process might look something like this:

1. Create a branch within the source code repository where the code and configuration is stored.
2. Edit and test your work in a development or sandbox org.
3. Push your changes to the new branch in the repository. Include configuration changes, code, unit tests, comments, etc.
4. Create a pull request and invite others to participate in the review.
5. Reviewers evaluate the work and make inline comments. Review is based on requirements documents, user stories, and coding standards. Complete initial review within 24 hours
6. If rework is required, return to step 2.
7. Once all reviewers have approved the changes, the work is merged into the repository, where it can be deployed to the next org in the process.

Review Best Practices

• Keep it small – 200 to 400 lines of code. This is what most people can review in 60-90 minutes.
• When requesting a review, add documentation so that the design is understandable.
• Take the time! When reviewing someone else’s work, take the time and effort to do a thorough job.
• Keep it positive. Besides pointing out defects, comment on good design and best practices.
• Use checklists
• Address and/or resolve all comments from reviewers
• Iterate (use findings to update checklists)

CODING STANDARDS
Well-written code is easy to read, understand, debug, and maintain. One key to attaining these attributes in your team’s code is to define coding standards – guidelines that may cover file organization, indentation, comments, declarations, statements, whitespace, naming conventions, programming practices, programming principles, architectural best practices, etc.

The following are example coding standards for languages typically used in Salesforce development. These should be modified to fit your team’s background, processes, and preferences. Be careful to avoid needless work and arguments over personal preferences. Make sure each standard is specific, unambiguous, and contributes to more readable, maintainable code.

Finally, note that these checklists are meant to be brief reminders. More verbose standards and samples should be provided to your development team.

Apex

• Use config instead of code when possible
• Clearly separate concerns (model / controller / trigger handler / view / tests)
• Avoid duplicate code – add base classes, utilities, subroutines, etc.
• Prevent SOQL injection
• Handle all potential errors; comment cases where errors should be ignored
• Provide useful and complete documentation for classes & methods
• Write small, clear, single-purpose methods
• Use descriptive names for classes, properties, methods, variables, etc.
• Use whitespace to improve readability
• CamelCase classes, ICamelCase interfaces, camelCase methods, properties, and variables, ALL_CAP finals
• Use spaces instead of tabs
• Make properties and parameters final when possible
• No SOQL, DML, or @future calls inside loops
• Use static queries, binding variables or escapeSingleQuotes to prevent SOQL injection attacks
• Bulkify all trigger and asynchronous code
• Use asynchronous code (future, batch, scheduled) when possible
• No hard-coded Ids
• Create at most one trigger per object
• Use centralized trigger processing (ITrigger pattern)
• Use custom settings, labels, or metadata types for configurable settings and preferences

Apex Unit Tests

• Assert proper behaviors
• Test conditionals, valid inputs, invalid inputs, error conditions
• Provide useful and complete documentation
• Avoid duplicate code – add base classes, utilities, subroutines, etc.
• System.runAs a specific user
• Write tests for multiple records
• Use mocking framework for callout testing
• Don’t rely on existing data
• 100% unit test coverage

Visualforce

• Use config instead of code when possible
• Avoid duplicate code – use components, includes, templates, etc.
• Provide useful and complete documentation
• Prevent cross-site-scripting attacks
• Move CSS and Javascript to static resources
• Include Javascript at the bottom of the page
• Use transient properties, read-only pages, and caching when possible
• Use whitespace and indenting to improve readability

Javascript

• Move Javascript to a static resource
• Load Javascript at the bottom of the page
• Wrap all code in a namespace or anonymous function to avoid collisions
• Avoid duplicate code – add utilities, subroutines, etc.
• Handle all potential errors; comment cases where errors should be ignored
• Provide useful and complete documentation
• Write small, clear, single-purpose methods
• Use descriptive names for methods, variables, etc.
• Use whitespace to improve readability
• Use spaces instead of tabs
• Declare all variables and functions