º£½ÇÌìÑÄÉçÇø

Good code quality is hard to define and even more difficult to measure. At the same time, a clean code culture - or rather the lack of one - can have significant cost and maintenance implications. At º£½ÇÌìÑÄÉçÇø, we’ve gone a very long way towards helping teams integrate automated code quality checks into their development pipelines. At the heart of this effort is , an awesome code analysis tool used in ³Ò±ð²¹°ù²õ±ð³Ù’s static code analysis. Our PMD-based code analyzer checks your codebase for issues in areas such as style, design, security, performance and best practice.

º£½ÇÌìÑÄÉçÇø, and our users, have benefitted hugely from PMD for Apex - we run static code analysis a few hundred thousand times a month, which catches an average of 60 violations per deployment. This wouldn’t be possible without the work of the PMD maintainers. So when , one of the maintainers of the open source PMD Source Code Analyzer Project, reached out to the Salesforce community for help and expertise, we were naturally more than happy to give something back to this fantastic project.

Good code is readable code

The challenges of maintaining a high-quality codebase are well known, of course. If you have large or distributed teams of developers with varying levels of experience and tight deadlines for shipping work, technical debt can quickly accumulate, as suboptimal code is added to the codebase under time pressure. In short, things can get complex and messy. You have to spend a lot of your time fixing bugs and trying to understand existing code before getting on with writing those great new features you had planned. To counteract this difficulty, we’ve been developing a new rule for the PMD code analyzer to flag up any Apex methods that are considered more difficult for humans to read and understand.

Making Apex methods easier to understand

According to psychologists, people can keep an average of 7 items (plus or minus 2) in their short-term memories. This means that, if you want to reduce code complexity, it makes sense to reduce the number of things your teammates need to remember while reviewing it. Here’s where measuring the cognitive complexity of a unit of code can help. Cognitive complexity is based on the following three principles:

1. A method is more complex if there’s a break in the control flow
2. A method is more complex if it contains nested control flows
3. A method isn’t made more complex by the use of language shorthands

In other words, each break in the control flow amounts to an additional piece of logic that you have to remember when trying to understand the code. Similarly, separate nested control flows are further pieces of the logic you need to keep in mind. Language shorthands, on the other hand, collapse multiple statements into one, in effect reducing the number of things you need to remember. Based on these constraints on human memory, cognitive complexity is a metric that counts the number of breaks in the linear control flow through the code, and assigns a single numerical score for each method.

Different ways of measuring complexity

With the upcoming release of PMD 6.22.0, the cognitive complexity metric will be included as a new rule for analyzing the control flow of Apex methods. As with existing PMD rules, you’ll be able to customize the settings for the new rule on the Account page within º£½ÇÌìÑÄÉçÇø under Static code analysis rulesets once we’ve incorporated settings for the new PMD rules into º£½ÇÌìÑÄÉçÇø:

Now, if you’re familiar with ³Ò±ð²¹°ù²õ±ð³Ù’s static code analysis ruleset, you might know that it already includes a rule for measuring a type of complexity, cyclomatic complexity. It’s worth understanding the , as both are helpful metrics for targeting methods that you should consider refactoring if you want to make your code more readable and maintainable.

‘Easy to read’ vs ‘easy to test’

Unlike cognitive complexity, which measures how easy it is for a human to understand code, cyclomatic complexity measures how easy it is to test code. In particular, cyclomatic complexity counts the number of unique paths contained in a method. Here’s an example:

public static StringToCommaSeparatedString(List<String> strings){
   return String.join(ids,',');
}

This simple method only has one unique path, and so it has the minimum possible cyclomatic complexity score of 1. But imagine you had a domain class with lots of small, simple methods like this one. The cyclomatic complexity score for that class would be high, even though the code, consisting of lots of simple methods, would be relatively easy for a developer to understand and maintain. On the other hand, if you wanted to test the code in the class, you’d have to write a test for each of its many (simple) methods.

Let’s now look at the above method in terms of cognitive complexity. Because the method doesn’t have any breaks in the linear control flow, it has a cognitive complexity score of 0 - it’s a simple method that’s easy to read. And if you had a class containing lots of simple methods like the above, the cognitive complexity score would still be low. What makes a class difficult to read is not that it contains a large number of methods, but large amounts of logic. For this reason, cognitive complexity is a useful metric to use at the class level, since classes containing a lot of logic will also have a high cognitive complexity score.

Suppressing code violation warnings

Static code analysis isn’t perfect. Sometimes it gets something wrong by failing to account for the context of the code it’s analysing. As an example, the following code would trigger ApexCRUDViolation errors from PMD:

public without sharing class AccountUpdater {
   public static void updateBigAcounts() {
       AccountUtilities.checkUpdateAccessForPriority();
       List<Account> accounts = [SELECT Id, Name FROM Account WHERE NumberOfEmployees > 1000];

       for (Account account : accounts) {
           account.CustomerPriority__c = 'high';
       }

       update accounts;
   }
}

public class AccountUtilities {
   public static void checkUpdateAccessForPriority() {
       if (!Schema.sObjectType.Account.fields.CustomerPriority__c.isUpdateable()) {
           throw new System.NoAccessException();
       }
   }
}

The errors are raised because PMD tries to detect that you’ve first checked that you have permission to read access an object before you attempt to access it. In this example, PMD fails to account for the method call checkUpdateAccessForPriority() in the second class above. For the moment at least, PMD fails here because it can’t follow method calls across class boundaries.

To prevent the ApexCRUDViolation errors, you might choose to suppress the PMD warnings on the lines that are causing the error. With the upcoming release of PMD 6.22.0, you’ll be able to do this by adding a NOPMD comment on those lines. It’s also good practice to include a brief comment to explain why you are suppressing the warning:

public without sharing class AccountUpdater {
   public static void updateBigAcounts() {
       AccountUtilities.checkUpdateAccessForPriority();
       List<Account> accounts = [SELECT Id, Name FROM Account WHERE NumberOfEmployees > 1000]; // NOPMD - access is checked in checkUpdateAccessForPriority

       for (Account account : accounts) {
           account.CustomerPriority__c = 'high';
       }

       update accounts; // NOPMD - Access is checked in checkUpdateAccessForPriority
   }
}

One ruleset to rule them all

Static code analysis rules are useful in most circumstances, but, as in the above example, occasionally they get it wrong. In those cases, it’s helpful to suppress the warnings locally with a NOPMD comment. You could just disable the rule, such as via the team settings for ³Ò±ð²¹°ù²õ±ð³Ù’s static code analysis. But we don’t recommend disabling individual rules globally in such cases because the rules help you and your teammates write correct, secure and performant code elsewhere within your project. After all, PMD integration within º£½ÇÌìÑÄÉçÇø is there to help keep your repository clean. That said, getting warnings from a PMD plugin in your IDE can also be very useful when iterating quickly. We recommend you use the same ruleset in º£½ÇÌìÑÄÉçÇø as you do in your IDE. Just export the º£½ÇÌìÑÄÉçÇø ruleset from the Static code analysis rulesets section on the Account page, and import those settings into your IDE.

Want to improve your code quality?

Making contributions to the improvement of PMD is our way of saying thank you to the maintainers of the PMD project for releasing such an important resource to the community.

³Ò±ð²¹°ù²õ±ð³Ù’s Salesforce code analysis tool runs throughout your release process, to make sure your code is high quality and consistent across the team. Start implementing best practice for static code analysis of your Apex on a of Gearset.