As a developer, dev lead, tech lead, or technical architect, have you ever had second thoughts about code — e.g., “Did that class have any error handling?” or “Did that test method have any system asserts?” — days after you’ve already approved it? We’ve all experienced those moments when reviewing mature code at a much later date and wondered why we made particular decisions or wished we had a user story reference. Luckily, these setbacks and code quality inconsistencies can be prevented with one simple process! Enter: developer checklists and code reviews.
If you want to make your code reviews more reliable (in conjunction with your code analysis tool), but have not yet implemented a process to achieve greater consistency, then this checklist is for you.
Note: You may remember our Developer Best Practices Checklist from 2015. The following is an updated checklist, which includes additional tools and topics from more recent Salesforce Platform releases.
Intended Audience: This is a code review checklist to be completed by developers, technical or dev leads, and technical architects while peer reviewing code.
Benefits: Code reviews help teams adhere to code standards in order to improve code quality and reduce bugs. Additionally, code reviews improve developer skills, build team culture, and encourage mentorship.
Suggested usage and measurement
- Review the following Developer Checklist as a team. Decide which sections apply to the codebase in your organization and which do not.
- If your team uses a code analysis tool, use this checklist after the code analysis tool.
- Developers: use this checklist to review and improve your own code after you’ve completed development.
- Code Reviewers: use this checklist while conducting a peer review.
- The checklist can also be used for a peer sign-off or approval workflow in version control before commits into release branch.
- To measure the Developer Checklist’s ROI, track the following before and after the checklist becomes a part of your development processes:
- The number of hours spent reviewing code
- The number and severity of issues logged, or defects opened
- The Salesforce org performance
Developer Checklist
Information and comments
Comments should always add value. Items marked “suggest” below indicate that they deviate from best practices; although they can add too much noise in code, they can also add value when working with junior developers.
- Top comment block description explains what the code does/business purpose
- If the code was modified only, the top comment block also explains why the code was modified
- Top comment block includes feature/story reference numbers (original feature/story AND any features/stories for modifications)
- Top comment block includes date the class was last modified by a developer (format: Dec-30-2011)
- Top comment block includes any other classes, flows, or process builders that are related
- Top comment block includes developer name that last modified the code
- Suggest that in-line comments are included on the line above variables explaining the variables
- Suggest that in-line that comments are included on the line above all SOQL queries explaining why query is required
- Suggest that in-line comments are included on the line above any loops explaining what the loop does and why it is required
- Suggest that in-line comments are included on the line above any DML explaining what is being committed and why
- Suggest that in-line comments are included on the line above what is being caught in try/catch and explain what is handled
- Suggest in-line comments are included on the line above limits being checked (if they are checked) and includes what limit are
Apex development
- Declarative development was used when applicable
- Solution could not be achieved declaratively
- Class uses invocable method if applicable
- Order of execution was reviewed if assignment rules, invocable, future, or queueable is being used (firing mechanism is low on list)
- Code is on latest API version
Trigger framework is followed
- Only one unmanaged trigger per object
- No logic is in the trigger
- No work is in the trigger-handler
- Work is in helper classes
- Handler is context-specific
- Article for trigger context
- The trigger/trigger handler prevents recursion
- Custom settings are used in bypass framework. Additional helpful logging framework here.
- Updates/changes to the current record in context should be done before insert/update context
Code is bulkified
- There are no SOQL queries in loops
- All SOQL queries are selective
- Code includes null checks
- How to query with null
- How to prevent null pointer exception
- Be aware of consequences of null
- Only use null check when necessary, better to use try/catch when possible
- There are no DML statements in loops
- Helper methods are bulkified
- Collections and queries are streamlined
- Strive for only one SOQL query instead of multiple queries by using relationships to query related objects/data
- Loops are efficient for large volume
- For loop includes query in DEFINITION of loop for large data
- Future/Queueable/Batch/Scheduled Apex are used appropriately and sparingly
- But why, you ask? All too often, we see async solutions implemented for the “wrong” reasons. Do NOT use async Apex when you are unsure of the full requirement, nor when transactional Apex will do the trick with a bit of refactoring. We sometimes fall into this trap because fresh async methods are easier to implement than to revisit a requirement or refactor existing code. This results in technical debt — do not do this.
- Instead:
- Respect limitations
- Scope the full requirements and use cases
- Implement the best solutions for the requirements
-
- Future follows guidelines
- Queueable follows guidelines
- Scheduled Apex follows guidelines
- Batch Apex follows guidelines
- Daisy-chaining classes
- Using Stateful and AllowCallouts
- Limits will not be reached
- The maximum number of future method invocations per a 24-hour period is 250,000, or the number of user licenses in your organization multiplied by 200 (whichever is greater). This limit is for your entire org and is shared with all asynchronous Apex: Batch Apex, Queueable Apex, Scheduled Apex, and Future methods.
- Custom Settings/Labels/Metadata are used
- No IDs, Names, Picklist Values, Descriptions, Strings, Numbers, etc are hard-coded
- Custom labels and hierarchical custom settings are used instead of hard coding
- Design makes use of custom metadata instead of custom settings when appropriate
- Code complies with security and checks permissions before executing DML
- With Sharing and Without Sharing keywords are avoided on domain Apex to ensure calling context drives this aspect
- With Sharing is preferred to enforce security as needed
- Security Best Practices are followed in setup
- Static Code Analysis via a third-party tool/PMD was run against the Apex development
- With Sharing and Without Sharing keywords are avoided on domain Apex to ensure calling context drives this aspect
- Best practice naming conventions are used
- Object name is included in class (trigger, handler, test class is named after class it is testing)
- Class name explains when it is used (helper)
- An error logging framework is used to catch and handle built-in exceptions, as well as custom exceptions and log errors
- Consider advanced error logging frameworks to catch rollback issues
- Integrations are reviewed by integration architect and follow integration best practices
- Callouts are NOT made on pageload unless approved by architect
- Integrated components are within subtab or accordion on pages unless approved by architect
Lightning Web Components
- Calls to server are limited (pass info between components when possible)
- Queries only contain columns in SELECT that are used/needed
- Queries have LIMIT set and use pagination when necessary
- Data is lazy loaded when possible and does not preload
- Lazy load locations were evaluated:
- Quick or Global Actions
- Utility Bar
- App Builder Tabs
- Use
aura:if
,lightning:tabset
andlightning:tab
when applicable
- Lazy load locations were evaluated:
- Data caching is used when possible and was evaluated
- Lightning Data Service was evaluated over using Apex to create, delete, and update records
- Storable Actions were evaluated
- Caching/LDS was evaluated
- Code uses static schema instead of dynamic schema when possible
- Code DOES NOT use pub/sub for parent-child or child-parent communication
- A parent component communicates with a child component by either:
- Setting the @api properties of the child (apiProperty recipe)
- Calling an @api method defined in the child component
- A child component communicates with its parent component by dispatching a DOM event with or without a data payload (eventWithData recipe and eventSimple recipe respectively).
- Always use CustomEvent (not Event), even when the event doesn’t have a data payload
- Prefer passing data using primitive data types in the event payload
- If you must pass data using a non-primitive data type in the event payload, pass a copy of the object or array to avoid leaking private objects and unpredictable mutations by the event listener
- If you need to pass a record, pass the record ID
- Use the Lightning Messaging channel to communicate across the DOM in the Lightning page;
c/pubsub
should only be used if there are limitations to LMS
- A parent component communicates with a child component by either:
- Use of third-party JavaScript libraries is limited or removed if possible
- Prefer Lightning SLDS (including images) to external images, lock image size (no high res)
- Use custom CSS customizer if unique look/feel is needed
- Code is inclusive and makes use of Accessibility Attributes
- Minimize number of times component is being rendered
- Code makes use of Canvas where applicable instead of iFrame; iFrames are sometimes easier but provide less functionality
- Code does not make use of
window.location
(see docs) - Aura components are migrated to LWC when possible
- Architect signs off when Aura component cannot be migrated and needs updated instead
- Lightning Component Inspector is used to monitor performance impact of new components
- The component follows LWC best practices
- Each LWC has a Jest test that tests the component’s behavior in isolation
Apex Test Classes
- Tests are meaningful
- Tests cover positive scenario
- Tests cover negative scenario
- Test cover null scenario
- Tests use custom settings instead of hard coding
- For bulk test (e.g., create xx records, insert xx records, assert xx records meet scenario)
- For setting/checking specific values from test scenarios (IDs, Names, Picklist Values, Descriptions, Numbers, etc.)
- Tests provide 90%+ code coverage
- If not able to achieve 90%, explanation is in comment block at top of class and additional sign off required
- A goal of 90% ensures a buffer for the 75% deployment requirement, and much less stressful deployments
- Tests make use of test factories/utils class instead of creating their own data
- Test data includes scenarios with both invalid and valid inputs, e.g. different data types or values
- Tests include System.Assert to validate expected outcomes
- Tests use runAs method to test in different user contexts and sharing
- Tests do not use SeeAllData
- Tests check bulk scenarios
- Tests use Order By to check the expected order
- Tests include comments (see comments section)
- Tests use startTest() and stopTest()
- Tests enforce Sharing
- Tests are not embedded in the main class
- Tests leverage mock testing framework to test web services
- Tests leverage @testSetup annotation for creating data that can be re-used in all test methods.
- Avoid creating test data for each test method individually
Performance
Code is written for performance requirements
- Integrations are asynchronous if there is not a real time requirement
- Integrations follow integration best practice
- Multiple requests are combined into single composite request when possible
- Queries were run through Query Planning Tool
- Caching was evaluated
- Salesforce Optimizer report was run
- The organization meets the Lightning Performance Technical Requirements
- All new custom components and changes have been monitored and there is not an uptick in EPT as result of change
- Additional approval is given if EPT is increased
- Performance limits are not reached
- OData Limits and Considerations have been reviewed by architect
- API Limits and Considerations have been reviewed by architect
- Process Limits and Considerations have been reviewed by architect
- Scheduled flows have been discussed with architect
- Invocable/scheduled/future/batch/queued Apex has been discussed with architect
- Email actions have been discussed with architect
Conclusion
Though this checklist is a tool meant to improve code reviews and unify teams of developers around best practices, it may also be good for individuals to review and reflect upon as they contribute to their codebase and grow their skills.
As a developer, dev lead, tech lead, or technical architect, were you aware of all of the best practices covered by the checklist? Hopefully this checklist served to help you remember important tips and tricks that you may have forgotten after months or years of focus elsewhere on the platform. There may be other governance processes at your company that can benefit from a checklist similar to this. We also suggest evaluating this checklist with your codebase to abbreviate or tailor it to your current needs.
Make this checklist yours and achieve better, more consistent development!
About the author
Shannon Tran is a 13x certified Technical Architect Director, Technical Consulting at Salesforce and current RAD Apex Coach. Prior to joining Salesforce, Shannon participated in the community as an author of Salesforce Finessed, as a Dreamforce presenter, and through various podcasts and at Trailblazer Community led events. She has been working with enterprise IT systems for over 10 years.