Apply these checks to every Apex class, trigger, and test file you write or review.
Scan for these patterns before declaring any Apex file acceptable:
// ❌ NEVER — causes LimitException at scale
for (Account a : accounts) {
List<Contact> contacts = [SELECT Id FROM Contact WHERE AccountId = :a.Id]; // SOQL in loop
update a; // DML in loop
}
// ✅ ALWAYS — collect, then query/update once
Set<Id> accountIds = new Map<Id, Account>(accounts).keySet();
Map<Id, List<Contact>> contactsByAccount = new Map<Id, List<Contact>>();
for (Contact c : [SELECT Id, AccountId FROM Contact WHERE AccountId IN :accountIds]) {
if (!contactsByAccount.containsKey(c.AccountId)) {
contactsByAccount.put(c.AccountId, new List<Contact>());
}
contactsByAccount.get(c.AccountId).add(c);
}
update accounts; // DML once, outside the loop
Rule: if you see [SELECT or Database.query, insert, update, delete, upsert, merge inside a for loop body — stop and refactor before proceeding.
Every class must declare its sharing intent explicitly. Undeclared sharing inherits from the caller — unpredictable behaviour.
| Declaration | When to use |
|---|---|
public with sharing class Foo |
Default for all service, handler, selector, and controller classes |
public without sharing class Foo |
Only when the class must run elevated (e.g. system-level logging, trigger bypass). Requires a code comment explaining why. |
public inherited sharing class Foo |
Framework entry points that should respect the caller's sharing context |
If a class does not have one of these three declarations, add it before writing anything else.
Apex code that reads or writes records on behalf of a user must verify object and field access. The platform does not enforce FLS or CRUD automatically in Apex.
// Check before querying a field
if (!Schema.sObjectType.Contact.fields.Email.isAccessible()) {
throw new System.NoAccessException();
}
// Or use WITH USER_MODE in SOQL (API 56.0+)
List<Contact> contacts = [SELECT Id, Email FROM Contact WHERE AccountId = :accId WITH USER_MODE];
// Or use Database.query with AccessLevel
List<Contact> contacts = Database.query('SELECT Id, Email FROM Contact', AccessLevel.USER_MODE);
Rule: any Apex method callable from a UI component, REST endpoint, or @InvocableMethod must enforce CRUD/FLS. Internal service methods called only from trusted contexts may use with sharing instead.
// ❌ NEVER — concatenates user input into SOQL string
String soql = 'SELECT Id FROM Account WHERE Name = \'' + userInput + '\'';
// ✅ ALWAYS — bind variable
String soql = [SELECT Id FROM Account WHERE Name = :userInput];
// ✅ For dynamic SOQL with user-controlled field names — validate against a whitelist
Set<String> allowedFields = new Set<String>{'Name', 'Industry', 'AnnualRevenue'};
if (!allowedFields.contains(userInput)) {
throw new IllegalArgumentException('Field not permitted: ' + userInput);
}
Prefer current language features (API 62.0 / Winter '25+):
| Old pattern | Modern replacement |
|---|---|
if (obj != null) { x = obj.Field__c; } |
x = obj?.Field__c; |
x = (y != null) ? y : defaultVal; |
x = y ?? defaultVal; |
System.assertEquals(expected, actual) |
Assert.areEqual(expected, actual) |
System.assert(condition) |
Assert.isTrue(condition) |
[SELECT ... WHERE ...] with no sharing context |
[SELECT ... WHERE ... WITH USER_MODE] |
Every feature must be tested across all three paths. Missing any one of these is a quality failure:
Test.startTest() / Test.stopTest() to isolate governor limit counters for async work.@isTest(SeeAllData=false) // Required — no exceptions without a documented reason
private class AccountServiceTest {
@TestSetup
static void makeData() {
// Create all test data here — use a factory if one exists in the project
}
@isTest
static void givenValidInput_whenProcessAccounts_thenFieldsUpdated() {
// Positive path
List<Account> accounts = [SELECT Id FROM Account LIMIT 10];
Test.startTest();
AccountService.processAccounts(accounts);
Test.stopTest();
// Assert meaningful outcomes — not just no exception
List<Account> updated = [SELECT Status__c FROM Account WHERE Id IN :accounts];
Assert.areEqual('Processed', updated[0].Status__c, 'Status should be Processed');
}
}
with sharing unless the trigger requires elevated access.| Pattern | Action |
|---|---|
SOQL inside for loop |
Refactor: query before the loop, operate on collections |
DML inside for loop |
Refactor: collect mutations, DML once after the loop |
| Class missing sharing declaration | Add with sharing (or document why without sharing) |
escape="false" on user data (VF) |
Remove — auto-escaping enforces XSS prevention |
Empty catch block |
Add logging and appropriate re-throw or error handling |
| String-concatenated SOQL with user input | Replace with bind variable or whitelist validation |
| Test with no assertion | Add a meaningful Assert.* call |
System.assert / System.assertEquals style |
Upgrade to Assert.isTrue / Assert.areEqual |
Hardcoded record ID ('001...') |
Replace with queried or inserted test record ID |