PMD (Programming Mistake Detector) is a static code analysis tool that helps Salesforce developers identify potential code quality, performance, and security issues in Apex. While PMD can detect numerous issues, not all are equally critical. This article will discuss the top 10 PMD issues that every Salesforce developer should focus on while writing Apex code.
Fixing these issues will improve code quality, performance, maintainability, and security.
1. Avoid SOQL Queries Inside Loops
This is the most critical issue in Salesforce Apex. While writing code, developers execute code with a limited set of records. So sometimes they don’t face issues even queries written in the loop.
Running SOQL queries inside a loop can cause governor limit violations. Salesforce enforces a limit of 100 SOQL queries per transaction, and if exceeded, the transaction will fail.
Bad Code Example
for (Account acc: accounts) {
List<Contact> contacts = [SELECT Id, Name FROM Contact WHERE AccountId = :acc.Id];
}
Best Practice (Fix)
Use bulkified queries outside the loop. We can get all account IDs and then execute a single SOQL to get all records with those account IDs. This will reduce the performance overhead.
Set<Id> accountIds=new Set<Id>();
for (Account acc: accounts) {
accountIds.add(acc.Id);
}
Map<Id, Contact> contactMap= new Map<Id, Contact>([SELECT Id,Name FROM Contact WHERE AccountId IN :accountIds]);
Refer to our post Optimizing Loop in Apex Code for handling loops in apex code.
2. Avoid DML Statements Inside Loops
Salesforce allows a maximum of 150 DML statements per transaction. Performing INSERT, UPDATE, DELETE, or UPSERT inside a loop can lead to governor-limit exceptions.
Bad Code Example
for (Opportunity opp: oppList) {
opp.Status='Closed';
update opp; // BAD PRACTICE
}
Best Practice (Fix)
Use bulk processing and perform DML operations outside the loop.
for (Opportunity opp: oppList) {
opp.Status='Closed';
}
update oppList; // GOOD PRACTICE
Check out our post Optimizing Salesforce Apex Code for handling DML inside the loop.
3. Avoid Hardcoded IDs
Hardcoding record IDs makes the code non-reusable and breaks deployments between different environments (Sandbox, UAT, Production).
Bad Code Example
List<Case> accs = [SELECT Id FROM Case WHERE RecordTypeId= '012300000012BYNQAG']; // BAD PRACTICEBest Practice (Fix)
Use custom metadata, custom settings, or dynamic queries to retrieve records dynamically.
CaseRecordTypeSetting__c recType = CaseRecordTypeSetting__c.getInstance();
List<Case> accs = [SELECT Id FROM Case WHERE RecordTypeId= :recType.CaseRecordType__c];
Check out our post Best Practices to Avoid Hardcoding in Apex for Cleaner Salesforce Code for best practices to avoid hardcoding in Apex.
4. Use Proper Exception Handling
Apex code without exception handling can result in uncaught exceptions, causing transaction failures or unwanted system exceptions. This can lead to poor user experience.
Bad Code Example
public void updateCase(Id recId) {
Case casRec = [SELECT Id,Email__c FROM Case where Id=:recId];
casRec.Email__c = 'test@example.com';
update casRec; // NO ERROR HANDLING
}
Best Practice (Fix)
Wrap the code inside a try-catch block to handle exceptions gracefully.
public void updateCase(Id recId) {
try {
Case casRec = [SELECT Id,Email__c FROM Case where Id=:recId];
casRec.Email__c = 'test@example.com';
update casRec;
} catch (DmlException e) {
System.debug('Error updating case: ' + e.getMessage()); // GOOD PRACTICE
}
}
Check out our post Exception Logging in Custom Object : Salesforce Apex for handling exceptions.
5. Use with Sharing for Security
Classes without with sharing keyword can access all records, potentially violating data security rules.
Bad Code Example
public class AccountHandler {
public List<Account> getAllAccounts() {
return [SELECT Id, Name FROM Account]; // EXPOSES ALL RECORDS
}
}
Best Practice (Fix)
Use with sharing to enforce user permissions. In this code, the user will only get records to which he/she has access.
public with sharing class AccountHandler {
public List<Account> getAllAccounts() {
return [SELECT Id, Name FROM Account with USER_MODE]; // GOOD PRACTICE
}
}
Check out our post Secure Apex Code with User Mode Operation for handling user mode operations.
6. Avoid Unused Variables and Methods
Unused variables and methods increase code complexity and affect readability. This will also increase the code line in Salesforce org.
Bad Code Example
public class TaxCalculationService {
private String recId; // UNUSED VARIABLE
public void calculateTestTax() {
} // UNUSED METHOD
}
Best Practice (Fix)
Remove unnecessary code to improve maintainability. It will also increase code lines that can be utilized for other features.
public with sharing class TaxCalculationService {
}Check out the post How to Manage Technical Debt in Salesforce to handle unwanted Code.
7. Avoid System.debug() in Production Code
Excessive System.debug() statements in production can cause log bloat and performance issues.
Bad Code Example
System.debug([select id from Account]); // SHOULD NOT BE IN PRODUCTION
Best Practice (Fix)
Use debug logs only for testing and remove them before deployment.
if (System.isDebug()) {
System.debug('This is for debugging only'); // GOOD PRACTICE
}
Check out the post Optimize Code by Disabling Debug Mode for handling debug mode.
8. Use Constants Instead of Hardcoded Values
Hardcoded values reduce code flexibility and maintainability.
Bad Code
public class DiscountCalculator {
public Double calculateDiscount(Double price) {
return price * 0.10; // HARDCODED VALUE
}
}
Best Practice (Fix)
Use constants for better maintainability. This way we can easily change value in the future and it will reflect in all places.
public class DiscountCalculator {
private static final Double DISCOUNT_RATE = 0.10;
public Double calculateDiscount(Double price) {
return price * DISCOUNT_RATE; // GOOD PRACTICE
}
}
Check out the post How to Manage Technical Debt in Salesforce to handle this issue.
9. Use Proper Variable Naming Conventions
Poorly named variables make code hard to read and understand. The Developer will forget use of this variable.
Bad Code
String a = 'John Doe'; // UNCLEAR VARIABLE NAME
Best Practice (Fix)
Use meaningful variable names for better readability. This will enhance developer productivity.
String customerName = 'John Doe'; // GOOD PRACTICE
Check out the post How to Manage Technical Debt in Salesforce to handle this issue.
10. Validate CRUD before DML Operation
CRUD operations can lead to unwanted DML operations. We should avoid DML operation if the user does not have access to record/object.
Account[] accts = [SELECT Id, Name FROM Account WHERE Name = 'Universal Containers' ALL ROWS];
delete accts;
We can utilize object and field permission before performing DML Operations. This will stop the DML operation if the er doesnot
Account[] accts = [SELECT Id, Name FROM Account WHERE Name = 'Universal Containers' ALL ROWS];
if (Schema.sObjectType.Account.isDeletable()) {
delete accts;
}
Check out the post Enforce Object-level and Field-level permissions in Apex to handle this issue.
Summary
PMD is an excellent tool for identifying critical code quality issues. It helps Salesforce Developers write secure, efficient, and more maintainable code. Ideally, Developers should run a PMD scan after every change in the Salesforce Apex code. Regularly running PMD scans on Apex code will prevent governor limit violations, security risks, and performance issues.
Related Posts
- How to Manage Technical Debt in Salesforce
- Optimizing Salesforce Apex Code
- DML or SOQL Inside Loops
- Limiting data rows for lists
- Disabling Debug Mode for production
- Stop Describing every time and use Caching of the described object
- Use Filter in SOQL
- Avoid Heap Size
- Optimize Trigger
- Bulkify Code
- Foreign Key Relationship in SOQL
- Defer Sharing Rules
- Avoid Hardcode in code
- Use Platform Caching
