Code review checklist to identify governor limits exceptions

Question

I am reviewing apex and visualforce code from a client org and would like to know if anybody has a code review checklist which is focussed on governor limits.

I want to identify key areas in triggers, controller classes, visualforce pages, web services which are potential bombshells for future governor limits.

Does anyone have a checklist for this or share one if you have? ​

Answer 1

Just check for any governed operation which occurs within a loop:

• Any query in a loop?
• Any DML in a loop?
• Any future calls in a loop?
• Any callout in a loop?
• Any email invocation in a loop?

Other than that, for CPU and Heap, you just have to keep an eye out for inefficient algorithms.

Automated tools such as PMD can find most of these for you. The trick is finding them when they're invoked indirectly, for instance through a method call.

Answer 2

In addition to the list posted by @AdrianLarson, there are other limits that you may need to be looking at in your code. You may also need to be reviewing to see if your code is testing to see if you're about to exceed a limit then preventing it from happening. This is particularly the case with asynchronous operations and can be used with other limits as well.

You can use the limits class to check things like getLimitQueueableJobs() and getQueueableJobs() to compare values and determine how close you are to exceeding limits before calling a new queuable. If close, you can abort/postpone the job if you don't have sufficient limits and create a mechanism to support that.

These are the kinds of practices you'll want to be looking for async code and in other code that may be part of service classes that are called frequently regardless of whether they occur in a loop. This is particularly true of email handlers (getEmailInvocations()), web services (getLimitCallouts()), batch operations that may use a lot of save points (getLimitSavepoints() and getLimitSavepointRollbacks()), and also could be important if you use Selector Classes that consume a lot of queries that can't cache results for reuse in maps (getLimitQueries()).

Answer 3

Great post! Here's the list I like to use:

General

• use config instead of code whenever possible
• DRY - don’t repeat yourself; use VF components, JS & Apex utilities and subroutines, etc.
• provide useful and complete documentation - field description / help text, Apex classes & methods, JS, etc.
• use descriptive names for objects, fields, classes, properties, variables, etc.

Config

• General standards above
• make custom objects and fields as restrictive as possible (required, unique, limited size, validation, precision, etc.)
• prevent duplicates using unique or validation rules
• normalize (no duplicate fields; delete unused fields)
• use camel case names with spaces between words (underscores in API names)
• add Apex unit tests to test config where appropriate

Apex

• General standards above
• no SOQL, DML, or @future calls inside loops
• bulkify all trigger and asynchronous code
• handle all potential errors; comment cases where errors should be ignored
• use asynchronous (future, batch, scheduled) when possible
• no hard-coded Ids
• when querying large data sets, use a SOQL “for” loop
• create at most one trigger per object
• use centralized trigger processing (ITrigger pattern)
• use static queries, binding variables or escapeSingleQuotes to prevent SOQL injection attacks
• clearly separate concerns (model / controller / trigger handler / view / tests)
• use custom settings for constants and configurable settings
• avoid text; use custom labels; use final properties
• avoid hard-coded date, time, number, currency formats
• when a group of fields is specified, use a field set
• follow Java coding conventions (for the most part)
• use spaces instead of tabs
• use whitespace to improve readability
• CamelCase classes, ICamelCase interfaces, camelCase methods, properties, and variables, ALL_CAP finals

Unit Testing

• General standards above
• assert proper behaviors
• test conditionals, valid inputs, invalid inputs
• always runAs a specific user
• write each test for multiple records
• don’t rely on existing data
• 100% unit test coverage

Visual Force

• General standards above
• 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

In addition to Salesforce's best practices and standards, there are lots of general software engineering practices you should be aware of. In my experience, some of the most important are:

• Design for testability
• Address each potential error
• DRY principal (don't repeat yourself)