Apex PMD common errors

Apex PMD (Programming Mistake Detector) as its name suggests, is a powerful tool designed to improve code quality for developers. It allows us to avoid maintenance and bug problems in our code by highlighting all the issues beforehand.

Setup Apex PMD

1. Use this Youtube video for a step by step installation guide
2. Create your own custom ruleset or use generic ones. I use this one.

Common Apex Errors

Below are some errors I come accross during my day to day work as a Salesforce developer and the solutions I use to avoid them.

1. ApexDoc
2. ExcessiveClassLength
3. AvoidDeeplyNestedIfStmts
4. ExcessiveParameterList
5. ApexCRUDViolation
6. ApexUnitTestClassShouldHaveAsserts

ApexDoc

Apex documentation is very important for code readability. Below is an example of apex headers for classes and methods.

/**
 * @author Akshi Boojhawon
 * @date 2024-04-01
 * @description Apex class to update Account Source
 */
public with sharing class UpdateAccountSource(){
/**
  * @author Akshi Boojhawon
  * @date 2024-04-01
  * @description updating account source to Web for Prospects
  * @param accType
  * @return List<Account> listUpdate
  */
  public static void updateAccountSourceWeb(String accType){
    List<Account> listAccUpdate = new List<Account>();

    for(Account acc : [SELECT Id, AccountSource FROM Account WHERE Type = :accType]){
      acc.AccountSource = 'Web';
      listAccUpdate.add(acc);
    }

    if(listAccUpdate.size() > 0){
      update listAccUpdate;
    }
    return listAccUpdate;
  }
}

ExcessiveClassLength

This error often shows then the class has over 1000 lines of code which can be broken down into external classes or functions. In breaking these methods apart the code becomes more managable and reuseable.

//Good Example
if(account.Type == 'Prospect' && account.AccountSource == 'Web' && account.Site == 'Apex Example'){
  UtilityClassProspect.manageProspects();
}
if(account.Type == 'Other'){
  UtilityClassOther.manageOthers();
}

AvoidDeeplyNestedIfStmts

Often, deeply nested if statements are often difficult to read. As an alternative, seperating each conditions into its own section can be more easy to read and manage.

//Bad Example
if(account.Type == 'Prospect'){
  if(account.AccountSource == 'Web'){
      if(account.Site == 'Apex Example'){
        //do logic here
      }
  }else if(account.AccountSource == 'Other'){
    //do logic here
  }
}

//Good Example
if(account.Type == 'Prospect' && account.AccountSource == 'Web' && account.Site == 'Apex Example'){
  //do logic here
}
if(account.Type == 'Other'){
  //do logic here
}

ExcessiveParameterList

Methods with multiple parameters are difficult to maintain and as an alternative solution in using multiple parameters, always store them in a collection. In my example, I'm using Map.

//Bad Example
public static void updateAccount(String accName,Integer accNumber,String accSource, String accType, Boolean isProspect, String accSite, Boolean isActive){
  //do logic here
}

//Good Example : Calling method from Apex
public static void updateAccount(Map<String,Object> mapAccInfo){
  String accName = mapAccInfo.get('accName');
  Integer accNumber = mapAccInfo.get('accNumber');
  //do logic here
}

//Good Example : Calling method from LWC
public static void updateAccount(String params){
  Map<String,Object> mapAccInfo = new Map<String,Object>();
  mapAccInfo = (Map<String,Object>)JSON.deserializeUntyped(params);
  String accName = (String)mapAccInfo.get('accName');
  Integer accNumber = (Integer)mapAccInfo.get('accNumber');
  //do logic here
}

ApexCRUDViolation

The rule triggers if you are checking for access permissions before a SOQL/SOSL/DML operation. Since Apex runs by default in system mode not having proper permissions checks results in escalation of privilege and may produce runtime errors. This check forces you to handle such scenarios. Below is an example on how to check for access for DML and SOQL operations.

//SOQL Operation
List<Account> listAccounts = [SELECT Id,AccountSource FROM Account WHERE Type = 'Prospect' WITH USER_MODE];

//DML Operation
if (Schema.sObjectType.Account.isUpdateable()) {
  update listAccounts;
}

ApexUnitTestClassShouldHaveAsserts

All apex test classes should have at least one System.assert method and for each assert, you should also have an assert message to explain what you do want to assert exactly and what is the outcome.

System.assertEquals(Expected result,Actual result, 'Asserting that Actual result is the same as expected result after update');

Reference

For more information on rules, refer to the Apex rules documentation.