The Basics of Clean Code in Apex

Part 58: The Basics of Clean Code in Apex

Welcome back to the Salesforce series. Up to this point, we have spent dozens of posts learning how to make Apex work. We have written triggers, services, batch classes, integrations, and tests. All of that matters. But there is a second skill that separates professional developers from hobbyists, and it has nothing to do with making code run. It is making code readable.

This is Part 58, and it is entirely about writing clean Apex. Clean code is code that another developer can open six months from now and understand in minutes, not hours. It is code that you yourself can return to after a long vacation and not feel lost. It is code that invites collaboration instead of creating fear.

We will cover five topics: why methods should be small, why methods should only do one thing, why naming matters more than you think, when and why you should leave comments, and how to set up PMD to enforce these practices automatically. There will be plenty of before-and-after examples so you can see the difference clean code makes.

Why Methods Should Be Small

The single most impactful thing you can do for code readability is to keep your methods short. A method that spans one hundred lines forces every reader to hold dozens of variables, conditions, and branches in their head at the same time. That is an unreasonable demand on human working memory.

The Ideal Method Length

There is no magic number, but aim for methods that fit on one screen without scrolling. In practice, that means somewhere between five and twenty lines for most methods. Some methods will be shorter. A few will be longer. The goal is not to hit a number. The goal is to make each method small enough that a reader can understand it in a single pass.

When a method grows beyond twenty or thirty lines, that is a signal. It does not mean the method is automatically bad. It means you should look at it and ask whether it is doing more than one thing.

Cognitive Load

Cognitive load is the amount of mental effort required to understand something. Every variable declaration, every conditional branch, every nested loop adds to the cognitive load of a method. When a method is small, the cognitive load is low. When a method is large, the cognitive load is high, and bugs hide in the complexity.

Consider what happens when you read a two-hundred-line method. By the time you reach line one hundred and fifty, you have forgotten what the variable on line twenty was doing. You scroll back up, re-read, scroll back down, and lose your place again. This is not a failure of intelligence. It is a failure of code organization.

Before: A Long Method

Here is a method that does too much. It validates input, queries records, performs calculations, creates new records, and sends notifications. All in one place.

public static void processOpportunities(List<Opportunity> opportunities) {
    List<Opportunity> validOpps = new List<Opportunity>();
    List<String> errors = new List<String>();

    for (Opportunity opp : opportunities) {
        if (opp.Amount == null || opp.Amount <= 0) {
            errors.add('Opportunity ' + opp.Name + ' has invalid amount.');
            continue;
        }
        if (opp.CloseDate == null) {
            errors.add('Opportunity ' + opp.Name + ' has no close date.');
            continue;
        }
        if (opp.CloseDate < Date.today()) {
            errors.add('Opportunity ' + opp.Name + ' has a past close date.');
            continue;
        }
        validOpps.add(opp);
    }

    if (!errors.isEmpty()) {
        System.debug('Validation errors: ' + errors);
    }

    Set<Id> accountIds = new Set<Id>();
    for (Opportunity opp : validOpps) {
        accountIds.add(opp.AccountId);
    }

    Map<Id, Account> accountMap = new Map<Id, Account>(
        [SELECT Id, Name, AnnualRevenue FROM Account WHERE Id IN :accountIds]
    );

    List<Task> followUpTasks = new List<Task>();
    List<Opportunity> oppsToUpdate = new List<Opportunity>();

    for (Opportunity opp : validOpps) {
        Account acc = accountMap.get(opp.AccountId);
        if (acc != null && acc.AnnualRevenue != null && acc.AnnualRevenue > 1000000) {
            opp.Priority__c = 'High';
        } else {
            opp.Priority__c = 'Normal';
        }
        oppsToUpdate.add(opp);

        Task t = new Task();
        t.Subject = 'Follow up on ' + opp.Name;
        t.WhatId = opp.Id;
        t.OwnerId = opp.OwnerId;
        t.ActivityDate = opp.CloseDate.addDays(-7);
        followUpTasks.add(t);
    }

    update oppsToUpdate;
    insert followUpTasks;
}

This method is about fifty lines, and it is already difficult to follow. It mixes validation logic, data access, business logic, and DML operations. A reader has to understand all of it at once.

After: Extracted Helper Methods

Here is the same functionality broken into small, focused methods.

public static void processOpportunities(List<Opportunity> opportunities) {
    List<Opportunity> validOpps = validateOpportunities(opportunities);
    Map<Id, Account> accountMap = queryRelatedAccounts(validOpps);
    assignPriorities(validOpps, accountMap);
    List<Task> followUpTasks = createFollowUpTasks(validOpps);

    update validOpps;
    insert followUpTasks;
}

private static List<Opportunity> validateOpportunities(List<Opportunity> opportunities) {
    List<Opportunity> validOpps = new List<Opportunity>();
    List<String> errors = new List<String>();

    for (Opportunity opp : opportunities) {
        String error = getValidationError(opp);
        if (error != null) {
            errors.add(error);
        } else {
            validOpps.add(opp);
        }
    }

    if (!errors.isEmpty()) {
        System.debug('Validation errors: ' + errors);
    }
    return validOpps;
}

private static String getValidationError(Opportunity opp) {
    if (opp.Amount == null || opp.Amount <= 0) {
        return 'Opportunity ' + opp.Name + ' has invalid amount.';
    }
    if (opp.CloseDate == null) {
        return 'Opportunity ' + opp.Name + ' has no close date.';
    }
    if (opp.CloseDate < Date.today()) {
        return 'Opportunity ' + opp.Name + ' has a past close date.';
    }
    return null;
}

private static Map<Id, Account> queryRelatedAccounts(List<Opportunity> opportunities) {
    Set<Id> accountIds = new Set<Id>();
    for (Opportunity opp : opportunities) {
        accountIds.add(opp.AccountId);
    }
    return new Map<Id, Account>(
        [SELECT Id, Name, AnnualRevenue FROM Account WHERE Id IN :accountIds]
    );
}

private static void assignPriorities(
    List<Opportunity> opportunities,
    Map<Id, Account> accountMap
) {
    for (Opportunity opp : opportunities) {
        opp.Priority__c = isHighValueAccount(accountMap.get(opp.AccountId))
            ? 'High'
            : 'Normal';
    }
}

private static Boolean isHighValueAccount(Account acc) {
    return acc != null
        && acc.AnnualRevenue != null
        && acc.AnnualRevenue > 1000000;
}

private static List<Task> createFollowUpTasks(List<Opportunity> opportunities) {
    List<Task> tasks = new List<Task>();
    for (Opportunity opp : opportunities) {
        Task t = new Task();
        t.Subject = 'Follow up on ' + opp.Name;
        t.WhatId = opp.Id;
        t.OwnerId = opp.OwnerId;
        t.ActivityDate = opp.CloseDate.addDays(-7);
        tasks.add(t);
    }
    return tasks;
}

Now the top-level method reads like a table of contents. You can see the entire workflow in six lines. If you need to understand validation, you read validateOpportunities. If you need to understand priority assignment, you read assignPriorities. You never have to hold the entire process in your head at once.

When to Extract a Method

Extract a method when you notice any of these signals:

• A comment explaining a block of code. If you need a comment to explain what a section does, that section should probably be its own method. The method name replaces the comment.
• A deeply nested block. Two or three levels of nesting is fine. Four or more is a sign that inner logic should be extracted.
• A repeated pattern. If you find yourself writing the same three lines in multiple places, extract them into a method.
• A logical grouping. If a block of lines work together to accomplish one sub-task, they belong in their own method.

Why Methods Should Only Do One Thing

Keeping methods small is the first step. The second step is making sure each method has a single, clear responsibility. This is the Single Responsibility Principle applied at the method level.

What Does “One Thing” Mean?

A method does one thing when you can describe what it does without using the word “and.” If you find yourself saying “this method validates the input and queries the database and calculates the discount,” that method is doing three things.

Compare these two descriptions:

• “This method processes opportunities.” (Vague. What does processing mean?)
• “This method assigns priority levels to opportunities based on their account’s annual revenue.” (Specific. One clear task.)

The second description tells you exactly what the method does, and it implies that the method does nothing else.

Side Effects

A side effect is when a method does something beyond what its name and return type suggest. Side effects are one of the most common sources of bugs because they violate the reader’s expectations.

// BAD: The method name says "validate" but it also modifies the record
public static Boolean validateOpportunity(Opportunity opp) {
    if (opp.Amount == null || opp.Amount <= 0) {
        return false;
    }
    // Side effect: modifying the record during validation
    opp.Description = 'Validated on ' + Date.today();
    opp.Validation_Status__c = 'Passed';
    return true;
}

A developer reading code that calls validateOpportunity will assume it only checks whether the Opportunity is valid. They will not expect it to modify the record. When they eventually discover the side effect, they will have to re-evaluate every place the method is called.

// GOOD: Validation only validates
public static Boolean isValidOpportunity(Opportunity opp) {
    return opp.Amount != null && opp.Amount > 0;
}

// Marking as validated is a separate, explicit operation
public static void markAsValidated(Opportunity opp) {
    opp.Description = 'Validated on ' + Date.today();
    opp.Validation_Status__c = 'Passed';
}

Now each method does exactly what its name says. No surprises.

Command-Query Separation

Command-Query Separation is a principle that says a method should either change state (a command) or return information (a query), but not both. This makes code easier to reason about because you can call query methods freely without worrying about side effects.

// BAD: This method both changes state and returns a value
public static Integer addLineItemAndReturnTotal(
    Order__c order,
    OrderItem__c item
) {
    item.Order__c = order.Id;
    insert item;
    // Now it also calculates and returns the total
    List<OrderItem__c> allItems = [
        SELECT Price__c FROM OrderItem__c WHERE Order__c = :order.Id
    ];
    Integer total = 0;
    for (OrderItem__c oi : allItems) {
        total += (Integer) oi.Price__c;
    }
    return total;
}

// GOOD: Separate command and query
public static void addLineItem(Order__c order, OrderItem__c item) {
    item.Order__c = order.Id;
    insert item;
}

public static Decimal calculateOrderTotal(Id orderId) {
    Decimal total = 0;
    for (OrderItem__c item : [
        SELECT Price__c FROM OrderItem__c WHERE Order__c = :orderId
    ]) {
        total += item.Price__c;
    }
    return total;
}

With the separated version, you can call calculateOrderTotal any time you need the total without worrying about inserting duplicate line items. And you can call addLineItem without being forced to receive a total you might not need.

Refactoring a Multi-Responsibility Method

Here is a trigger handler method that does too many things.

// BEFORE: Does validation, enrichment, and notification
public static void handleBeforeInsert(List<Case> newCases) {
    for (Case c : newCases) {
        // Validation
        if (String.isBlank(c.Subject)) {
            c.addError('Subject is required.');
        }
        if (c.ContactId == null && c.SuppliedEmail == null) {
            c.addError('Either Contact or Email is required.');
        }
        // Enrichment
        if (c.Origin == null) {
            c.Origin = 'Web';
        }
        if (c.Priority == null) {
            c.Priority = 'Medium';
        }
        // Auto-assignment
        if (c.Type == 'Billing') {
            c.OwnerId = Label.Billing_Queue_Id;
        } else if (c.Type == 'Technical') {
            c.OwnerId = Label.Technical_Queue_Id;
        }
    }
}

// AFTER: Each responsibility is its own method
public static void handleBeforeInsert(List<Case> newCases) {
    validateRequiredFields(newCases);
    applyDefaults(newCases);
    assignToQueues(newCases);
}

private static void validateRequiredFields(List<Case> cases) {
    for (Case c : cases) {
        if (String.isBlank(c.Subject)) {
            c.addError('Subject is required.');
        }
        if (c.ContactId == null && c.SuppliedEmail == null) {
            c.addError('Either Contact or Email is required.');
        }
    }
}

private static void applyDefaults(List<Case> cases) {
    for (Case c : cases) {
        if (c.Origin == null) {
            c.Origin = 'Web';
        }
        if (c.Priority == null) {
            c.Priority = 'Medium';
        }
    }
}

private static void assignToQueues(List<Case> cases) {
    for (Case c : cases) {
        if (c.Type == 'Billing') {
            c.OwnerId = Label.Billing_Queue_Id;
        } else if (c.Type == 'Technical') {
            c.OwnerId = Label.Technical_Queue_Id;
        }
    }
}

The refactored version is longer in total lines, and that is fine. The goal is not fewer lines. The goal is that each piece of logic lives in one place, has one name, and can be understood independently.

Why Naming Things Well Is Critical

Phil Karlton famously said there are only two hard things in computer science: cache invalidation and naming things. He was right. Naming is hard because a good name has to compress an entire concept into a few words. But it is worth the effort because names are the primary way developers understand code.

Class Naming Conventions

In Apex, classes should use PascalCase and describe what the class is or does. Salesforce has common suffixes that communicate architectural roles:

• Service classes: OpportunityService, AccountService — contain business logic
• Selector classes: OpportunitySelector, CaseSelector — contain SOQL queries
• Trigger handlers: OpportunityTriggerHandler, CaseTriggerHandler
• Test classes: OpportunityServiceTest, CaseTriggerHandlerTest
• Utility classes: DateUtils, StringUtils — contain stateless helper methods
• Domain classes: Opportunities, Cases — represent collections with behavior
• Controllers: InvoiceController, RegistrationController

Avoid generic names like Helper, Manager, Processor, or Handler without a prefix. AccountHelper is slightly better than Helper, but AccountAddressValidator is better still because it tells you exactly what the class does.

Method Naming Conventions

Methods should use camelCase and start with a verb that describes the action being performed.

// BAD: Vague or noun-based names
public static List<Account> accounts() { ... }
public static void data(List<Contact> contacts) { ... }
public static String result(Opportunity opp) { ... }

// GOOD: Verb-first names that communicate intent
public static List<Account> queryActiveAccounts() { ... }
public static void enrichContactAddresses(List<Contact> contacts) { ... }
public static String calculateDiscountTier(Opportunity opp) { ... }

Common verb prefixes and what they communicate:

Variable Naming

Variables should describe what they hold, not their type or a cryptic abbreviation.

// BAD: Abbreviations and meaningless names
List<Account> aList = new List<Account>();
Map<Id, Contact> m = new Map<Id, Contact>();
Integer x = 0;
String s = opp.Name;
Boolean b = true;

// GOOD: Descriptive names
List<Account> activeAccounts = new List<Account>();
Map<Id, Contact> contactsByAccountId = new Map<Id, Contact>();
Integer retryCount = 0;
String opportunityName = opp.Name;
Boolean isEligibleForDiscount = true;

Notice the map name contactsByAccountId. This pattern — [value type]By[key description] — is extremely useful for maps because it tells you both what the map contains and how it is keyed.

Constants

Constants should use UPPER_SNAKE_CASE and live in a location that makes their scope clear.

// BAD: Magic numbers and strings scattered through code
if (opp.Amount > 100000) { ... }
if (acc.Type == 'Enterprise') { ... }

// GOOD: Named constants
private static final Decimal HIGH_VALUE_THRESHOLD = 100000;
private static final String ACCOUNT_TYPE_ENTERPRISE = 'Enterprise';

if (opp.Amount > HIGH_VALUE_THRESHOLD) { ... }
if (acc.Type == ACCOUNT_TYPE_ENTERPRISE) { ... }

Named constants eliminate the “what does this number mean?” question and ensure that when the threshold changes, you only change it in one place.

Boolean Naming

Booleans should read like a yes-or-no question. Use prefixes like is, has, can, should, or was.

// BAD: Ambiguous boolean names
Boolean active = true;
Boolean email = false;
Boolean process = true;

// GOOD: Names that read as questions
Boolean isActive = true;
Boolean hasEmailAddress = false;
Boolean shouldProcessRecord = true;

When you read if (shouldProcessRecord), the intent is immediately clear. When you read if (process), you have to figure out whether process is a noun (a thing) or a verb (an action) or an adjective (a state).

Avoiding Abbreviations

Abbreviations save a few keystrokes and cost hours of confusion. Resist the temptation.

// BAD
Integer accCnt = 0;
String oppStg = opp.StageName;
List<Contact> ctcts = new List<Contact>();
Map<Id, Account> accMap = new Map<Id, Account>();

// GOOD
Integer accountCount = 0;
String opportunityStageName = opp.StageName;
List<Contact> contacts = new List<Contact>();
Map<Id, Account> accountsById = new Map<Id, Account>();

The only abbreviations that are acceptable are ones that are more widely known than their full form: Id, URL, API, HTTP. If there is any chance someone will not know the abbreviation, spell it out.

Self-Documenting Code vs Comments

When your names are good enough, comments become unnecessary for explaining what the code does. The code explains itself.

// BAD: Comment compensating for a bad name
// Check if the account is a high-value enterprise account
if (a.Type == 'Enterprise' && a.AnnualRevenue > 1000000) { ... }

// GOOD: The method name IS the explanation
if (isHighValueEnterpriseAccount(acc)) { ... }

private static Boolean isHighValueEnterpriseAccount(Account acc) {
    return acc.Type == 'Enterprise' && acc.AnnualRevenue > 1000000;
}

In the second version, you do not need a comment. The method name tells you exactly what the condition checks. This is self-documenting code.

When and Why You Should Leave Comments

If good names reduce the need for comments, does that mean you should never comment? No. It means you should be intentional about when you comment and what you say. A well-placed comment can be invaluable. A poorly placed comment is worse than no comment at all.

Good Comments

Intent comments explain why you made a decision, not what the code does. The code already shows what it does. The comment provides context that the code cannot.

// We process Accounts before Contacts because Contact validation
// depends on Account.BillingCountry, which may be updated in the
// same transaction.
processAccounts(scope);
processContacts(scope);

Without this comment, a future developer might reorder the method calls for aesthetic reasons and introduce a subtle bug. The comment prevents that.

Legal and compliance comments are sometimes required by your organization or by regulations.

// GDPR: Personal data fields are cleared after 24 months of inactivity
// per Data Retention Policy DRP-2024-003, approved 2025-01-15.
private static void purgeInactiveContactData(List<Contact> contacts) { ... }

TODO comments mark work that needs to be done but cannot be done right now. They should include a reason and ideally a ticket number.

// TODO (JIRA-4521): Replace hardcoded queue ID with Custom Metadata lookup
// once the metadata type is deployed to production.
private static final Id SUPPORT_QUEUE_ID = '00G...';

Warning comments alert other developers to non-obvious consequences.

// WARNING: This method performs a callout. Do not call it from a
// before-trigger context or inside a loop.
public static HttpResponse fetchCreditScore(String taxId) { ... }

Javadoc-style comments for public APIs and service methods help other developers use your code correctly without reading the implementation.

/**
 * Assigns territory-based owners to a list of Leads.
 * Leads without a valid State/Province are skipped and logged.
 *
 * @param leads The Leads to process. Must not be null.
 * @return The number of Leads that were successfully assigned.
 */
public static Integer assignTerritoryOwners(List<Lead> leads) { ... }

Bad Comments

Obvious comments restate what the code already says. They add visual noise without adding understanding.

// BAD: Every one of these comments is unnecessary

// Declare a list of accounts
List<Account> accounts = new List<Account>();

// Loop through the contacts
for (Contact con : contacts) {
    // Get the account ID
    Id accountId = con.AccountId;
    // Add to set
    accountIds.add(accountId);
}

// Insert the records
insert accounts;

Remove all of these. The code is perfectly clear without them.

Changelog comments belong in version control, not in the source file. Your git history records who changed what and when. Embedding that information in comments creates maintenance overhead and the comments quickly become outdated.

// BAD: Changelog in code
// 2024-03-15 - J. Smith - Added null check for Amount
// 2024-04-02 - A. Jones - Fixed bug with close date validation
// 2024-06-18 - J. Smith - Added priority assignment logic
public static void processOpportunities(List<Opportunity> opps) { ... }

Delete the changelog. Use git log and git blame instead.

Commented-out code is dead weight. Developers are afraid to delete it because they think they might need it later. But version control already preserves every line that was ever written. Commented-out code confuses readers, clutters the file, and creates doubt about whether it should be there.

// BAD: Zombie code
public static void assignOwner(Case c) {
    // if (c.Type == 'Billing') {
    //     c.OwnerId = getBillingQueueId();
    // }
    // Old logic - keeping in case we need to revert
    // c.OwnerId = getDefaultQueueId();
    c.OwnerId = getQueueIdByType(c.Type);
}

Delete the commented-out lines. If you need the old logic, it lives in git.

When Code Should Speak for Itself

If you feel the urge to write a comment, first ask: can I rename something to make this comment unnecessary?

// BAD: Comment needed because the code is unclear
// Check if we should skip this record
if (rec.Status__c == 'Closed' || rec.IsDeleted || rec.CreatedDate < cutoff) {
    continue;
}

// GOOD: Extract a method and let the name speak
if (shouldSkipRecord(rec, cutoffDate)) {
    continue;
}

private static Boolean shouldSkipRecord(MyObject__c rec, Date cutoffDate) {
    return rec.Status__c == 'Closed'
        || rec.IsDeleted
        || rec.CreatedDate < cutoffDate;
}

The method name eliminates the need for a comment. The method body is clear enough to stand on its own. This is the ideal: code that communicates its purpose through structure and naming, with comments reserved for the rare cases where context cannot be encoded in the code itself.

How to Set Up and Use PMD

You can write clean code through discipline and code review, but humans are inconsistent. We forget rules, miss things when we are tired, and have blind spots. That is why static analysis tools exist. PMD is the most widely used static analysis tool for Apex, and it catches problems before your code ever runs.

What Is PMD?

PMD is an open-source static analysis tool that scans source code and flags potential problems based on a configurable set of rules. It does not execute your code. It reads the source files, builds an abstract syntax tree, and checks that tree against rules like “methods should not exceed fifty lines” or “avoid SOQL queries inside loops.”

PMD supports many languages, and Apex is one of them. The Apex rules in PMD cover security, performance, style, best practices, and testing standards.

Why Static Analysis Matters

Static analysis catches problems that are easy for humans to overlook:

• A SOQL query inside a for loop that works in dev but hits governor limits in production
• An empty catch block that silently swallows exceptions
• A method that has grown to two hundred lines without anyone noticing
• A test class with no assertions
• An unused variable that clutters the code
• Hardcoded IDs that will break in a different org

These are not theoretical risks. They are the bugs you will actually encounter in real Salesforce projects. PMD catches them automatically, before code review, before deployment, and before production incidents.

Installing PMD for Apex

PMD is a Java application. To install it on your local machine:

1. Make sure Java is installed. PMD requires Java 8 or higher. Check with java -version in your terminal.
2. Download PMD. Go to the PMD releases page on GitHub and download the latest binary distribution (the zip file).
3. Extract the archive. Unzip it to a location on your machine, for example /opt/pmd or C:\pmd.
4. Add PMD to your PATH (optional but recommended). Add the bin directory to your system PATH so you can run pmd from any directory.

Verify the installation:

pmd --version

You should see the version number printed to the console.

The VS Code PMD Extension

If you use VS Code with the Salesforce Extension Pack (and you should), you can integrate PMD directly into your editor. The extension is called Apex PMD.

1. Open VS Code.
2. Go to the Extensions panel (Ctrl+Shift+X or Cmd+Shift+X).
3. Search for Apex PMD.
4. Click Install.

Once installed, PMD runs automatically every time you save an Apex file. Violations appear as yellow warning underlines in the editor and in the Problems panel at the bottom. You get feedback in real time, while you are still writing the code, which is when it is cheapest to fix.

You can configure the extension in your VS Code settings:

{
    "apexPMD.pmdBinPath": "/opt/pmd/bin",
    "apexPMD.rulesets": [
        "${workspaceFolder}/pmd-ruleset.xml"
    ],
    "apexPMD.runOnFileOpen": true,
    "apexPMD.runOnFileSave": true
}

Running PMD from the Command Line

For CI/CD pipelines and batch analysis, you run PMD from the terminal.

pmd check \
    --dir force-app/main/default/classes \
    --rulesets category/apex/bestpractices.xml,category/apex/security.xml \
    --format text

This command scans all Apex classes in the force-app directory against the best practices and security rule sets, and outputs the results as plain text.

You can change the output format to CSV, JSON, XML, or HTML depending on your needs:

pmd check \
    --dir force-app/main/default/classes \
    --rulesets category/apex/bestpractices.xml \
    --format csv \
    --report-file pmd-results.csv

Understanding PMD Rule Categories

PMD organizes its Apex rules into categories. Here are the most important ones.

Best Practices (ApexUnit) — Rules that enforce good testing habits.

Performance — Rules that prevent governor limit issues.

Security — Rules that catch common security vulnerabilities.

Style — Rules that enforce consistent code formatting and structure.

Customizing Your Ruleset

The default rules will not be a perfect fit for every project. Some rules may be too strict for your team. Others may not apply to your architecture. PMD lets you create a custom ruleset XML file that includes exactly the rules you want.

Here is an example custom ruleset:

<?xml version="1.0" encoding="UTF-8"?>
<ruleset name="My Project Rules"
    xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0
        https://pmd.sourceforge.io/ruleset_2_0_0.xsd">

    <description>Custom Apex ruleset for our Salesforce project</description>

    <!-- Include all best practice rules -->
    <rule ref="category/apex/bestpractices.xml" />

    <!-- Include all security rules -->
    <rule ref="category/apex/security.xml" />

    <!-- Include performance rules -->
    <rule ref="category/apex/performance.xml" />

    <!-- Include style rules but exclude a few -->
    <rule ref="category/apex/design.xml">
        <exclude name="ExcessiveParameterList" />
    </rule>

    <!-- Override method length threshold -->
    <rule ref="category/apex/design.xml/ExcessiveMethodLength">
        <properties>
            <property name="minimum" value="60" />
        </properties>
    </rule>

    <!-- Override class length threshold -->
    <rule ref="category/apex/design.xml/ExcessiveClassLength">
        <properties>
            <property name="minimum" value="600" />
        </properties>
    </rule>

</ruleset>

Save this file as pmd-ruleset.xml in your project root, then reference it when running PMD:

pmd check \
    --dir force-app/main/default/classes \
    --rulesets pmd-ruleset.xml \
    --format text

Fixing Common PMD Violations

Let us walk through the most common violations you will encounter and how to fix them.

AvoidSoqlInLoops — This is the violation you will see most often.

// VIOLATION: SOQL inside a loop
for (Contact con : contacts) {
    Account acc = [SELECT Id, Name FROM Account WHERE Id = :con.AccountId];
    con.Description = 'Account: ' + acc.Name;
}

// FIX: Query once, use a map
Set<Id> accountIds = new Set<Id>();
for (Contact con : contacts) {
    accountIds.add(con.AccountId);
}
Map<Id, Account> accountsById = new Map<Id, Account>(
    [SELECT Id, Name FROM Account WHERE Id IN :accountIds]
);
for (Contact con : contacts) {
    Account acc = accountsById.get(con.AccountId);
    if (acc != null) {
        con.Description = 'Account: ' + acc.Name;
    }
}

ApexSharingViolations — Every Apex class should declare its sharing model explicitly.

// VIOLATION: No sharing declaration
public class OpportunityService {
    // ...
}

// FIX: Declare sharing explicitly
public with sharing class OpportunityService {
    // ...
}

ApexUnitTestClassShouldHaveAsserts — Tests must assert something. A test that runs code without checking results proves nothing.

// VIOLATION: No assertions
@isTest
static void testCreateAccount() {
    Account acc = new Account(Name = 'Test');
    insert acc;
}

// FIX: Assert the expected outcome
@isTest
static void testCreateAccount() {
    Account acc = new Account(Name = 'Test');
    insert acc;

    Account result = [SELECT Id, Name FROM Account WHERE Id = :acc.Id];
    System.assertEquals('Test', result.Name, 'Account name should match');
}

EmptyCatchBlock — Empty catch blocks hide errors and make debugging nearly impossible.

// VIOLATION: Empty catch
try {
    callExternalService();
} catch (Exception e) {
    // do nothing
}

// FIX: At minimum, log the error
try {
    callExternalService();
} catch (Exception e) {
    Logger.error('External service call failed', e);
}

Integrating PMD into CI/CD

The real power of PMD comes when you run it automatically on every pull request. This way, violations are caught before code is merged, and the team maintains consistent standards without relying on manual reviews to catch every issue.

GitHub Actions example:

name: Apex Static Analysis

on:
  pull_request:
    paths:
      - 'force-app/**/*.cls'
      - 'force-app/**/*.trigger'

jobs:
  pmd:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4

      - name: Set up Java
        uses: actions/setup-java@v4
        with:
          distribution: 'temurin'
          java-version: '17'

      - name: Download PMD
        run: |
          wget https://github.com/pmd/pmd/releases/download/pmd_releases%2F7.0.0/pmd-dist-7.0.0-bin.zip
          unzip pmd-dist-7.0.0-bin.zip

      - name: Run PMD
        run: |
          pmd-dist-7.0.0/bin/pmd check \
            --dir force-app/main/default/classes \
            --rulesets pmd-ruleset.xml \
            --format text \
            --fail-on-violation true

With --fail-on-violation true, the pipeline fails if any violations are found, preventing the pull request from being merged until the issues are resolved.

You can also use dedicated GitHub Actions like pmd/pmd-github-action which annotates the pull request diff directly with violation comments, making it easy for developers to see exactly where the problems are.

Sfdx Scanner — Salesforce also provides sfdx scanner, a CLI plugin that wraps PMD along with other analysis engines. If your team already uses the Salesforce CLI, this can be a simpler option:

sf scanner run \
    --target force-app/main/default/classes \
    --category "Best Practices,Security,Performance" \
    --format table

The scanner plugin comes with Apex rules preconfigured, so you can get started without creating a custom ruleset.

Putting It All Together

Clean code is not about following rules for the sake of rules. It is about empathy for the next person who reads your code, and that next person is often future you. Here is a summary of the practices from this post:

1. Keep methods small. If a method does not fit on one screen, look for ways to extract helper methods.
2. Give each method one job. If you need the word “and” to describe what a method does, split it up.
3. Name things with intent. A good name eliminates the need for a comment. Invest the time.
4. Comment the why, not the what. Let the code explain what it does. Use comments to explain why it does it.
5. Delete dead code. Commented-out code, unused variables, and changelog comments belong in version control, not in the source file.
6. Use PMD. Automate what can be automated. Run PMD in your editor and in your CI/CD pipeline so violations are caught early.

These practices compound over time. A codebase where every developer follows them stays navigable as it grows from ten classes to a thousand. A codebase where nobody follows them becomes a liability.

What Is Next?

In Part 59, we will explore Agentforce for Developers. Agentforce is Salesforce’s AI-powered platform for building autonomous agents, and it opens up entirely new patterns for how developers build on the platform. We will look at what Agentforce is, how agents work under the hood, and how developers can extend Agentforce with custom Apex actions. See you there.