Clean Apex Code: Comments don't lie, developers do

When you find dead code, do the right thing. Give it a decent burial. Delete it from the system. —Robert C. Martin

How to properly use comments and when to avoid them is cause for endless debate in online communities. In this chapter, I start by sharing my opinion on 2 popular misconceptions about comments. Then, I go over different types of comments and explain their pros and cons.

💡
This is an preview of my upcoming book: Clean Apex Code, Software Design for Salesforce Developers

6.1 Everybody lies

If you’ve been programming for a while, you may have heard developers saying that code should be self-documenting and that comments shouldn’t be used because they tell lies. This idea comes from the very real fact that compilers don’t check comments for validity, so you could end up with something like this:

// insert the account record
update opportunity;

Clearly, the comment is saying one thing but the code does another one completely different; in other words, the comment is telling lies. And to state the obvious, the code’s logic is what will actually be executed, not what the comment says. Clean code purists use this argument to say comments should hardly ever be used.

In reality, any programming construct can tell lies or be misleading, consider the following code

void deleteOpportunity(Account caseRecord) {
     insert new Contact(LastName = 'FirstName');
}

This may be a pedantic example, but the point is that everything about this method is a lie. The method does not delete an opportunity, it does not take a case record as it’s parameter, and ends up actually inserting a contact. All these lies and yet there isn’t a single comment here.

So, it’s not the comments lie by default, is that it’s easy for programmers to not pay attention to the names they are using in their programming constructs. All the practices we explored in chapter 2 should help you avoid misleading other developers.

Now, what can happen, is that comments can often fall out of sync with the code that they once supported. When the code changes and the comments are not updated, they start telling lies and misleading. But this is also true for code; consider the example where a developer writes a method to deactivate an account

public void deactivateAccount(Account account) {

    account.IsActive = false,
    update account;
}

Imagine that a few months later, the business requests that when an account is deactivated, contacts should be deleted too, and the developer does the following

public void deactivateAccount(Account account) {

    account.IsActive = false,
    update account;
    
    delete [SELECT Id FROM Contact WHERE AccountId = :account.Id]
}

Now, the method does two very different things. A naive developer may reuse the deactivateAccount method in places where he did not intend for contacts to be deleted. So, this method lies by omission. It says it will deactivate an account but it also deletes its contacts.

A better approach would have been something like this

public void deactivateAccount(Account account, Boolean deleteContacts) {
   ...
}

Or

public void deactivateAccountWithContactDeletion(Account account) {
   ...
}

In chapter 3, we spoke extensively about how to properly structure methods to do multiple things without breaking the “do one thing” principle. The point of this example is that the method, once accurate, started telling lies as soon as the business requirements changed.

So, all programming constructs can lie. The onus is on us as developers to keep them updated as our code evolves.

6.2 Self-documenting code

The next idea that is popular in the developer community is that code should be self-documenting, i.e that the code itself is the documentation and that comments shouldn’t be needed to explain the code. I think it’s not that simple.

To illustrate this, consider this method from the Nebula Logger open-source Apex library, which is popular at the time of this writing for logging and exception management in Apex

global void finish(Database.BatchableContext batchableContext) {
        Logger.setAsyncContext(batchableContext);
        Logger.setParentLogTransactionId(this.originalTransactionId);
        LoggerBatchableContext input = new LoggerBatchableContext(batchableContext, this.currentSObjectType);
        this.executePlugins(BatchableMethod.FINISH, input, null);

        Id nextBatchJobId;
        if (this.currentSObjectType != Schema.Log__c.SObjectType) {
            nextBatchJobId = Database.executeBatch(this, this.chainedBatchSize);
        }
        
        ...more code here
        
}

Spend a few seconds reading this and think if you can decipher with 100% certainty what this code is doing, why and what purposes it serves. If you are stuck, try again, now by adding the comment that the developer added just above the method

/**
 * @description Required by the Database.Batchable interface.  
 * This method runs after all batch jobs are complete.
 * This method writes a status to the Log__c object indicating that 
 * the purge has finished.
 * @param batchableContext - The context of the batch jobs
*/

Was it any easier?

The point I’m trying to make here is that the names of our programming constructs can only take us so far. Most of the times, to be able to make sense of a piece of code, you need:

  • To really understand the context in which the code lives (is it within a method, or a larger class)
  • The overall purpose of the project or product
  • The problem domain
  • Any specific limitations or constraints that the product is working against
  • The previous design or architectural decisions that influence every aspect of the code base

Now, this doesn’t mean we shouldn’t try to make code as self-documenting as possible. Good names (chapter 2) and deep methods (chapter 4) can go a long way in ensuring the code does what it says it does.

Having explored these 2 misconceptions (comments lie and code can be 100% self-documenting), let’s explore different types of comments.

6.3 Version control comments

In Apex code bases, it’s very common to see comments that aim to support some sort of version control. For example

/**
 * History
 * 2021-06-30 - [BUG-452] - Rahul.Patel - Initial method for geocoding.
 * 2021-07-01 - [ENH-89] - Maria.Sanchez - Added state handling.
 * 2021-07-02 - [DEVOPS-2235] - Alex.Wu - Added postal code validation.
 * 2021-07-03 - [DEVOPS-2236] - Priya.Kumar - Support for country-specific addresses.
 * 2021-07-04 - [DEVOPS-2237] - David.Johnson - Minor refactor for performance.
 * 2021-07-05 - [DEVOPS-2238] - James.Lee - Fixed typo and updated international logic.
 */
@InvocableMethod(callout=true label='Geocode address')
public static List<Coordinates> geocodeAddresses(...)

This is a huge anti-pattern. However, I will play devil’s advocate for a while as there are some nuances to why this isn’t ideal. The typical argument against these type of comments is that they shouldn’t be used because you should use Git instead. While that’s true, saying “you should use Git for that” isn’t helpful. I’ve dedicated the last 3 years of my Salesforce career to Salesforce DevOps and I can tell you there are many teams that aren’t yet using version control. Now, it’s clear that they should, but because of different circumstances, many developers find themselves working in an environment where version control isn’t yet configured. Here are some reasons that come to mind:

  • Until recently, the org was very simple and they didn’t have developers. There’s only a newly hired developer in the team and they don’t know how to set up version control end-to-end.
  • Perhaps a company hires a developer as a contractor for a few months, and because of compliance and security reasons, the developer isn’t given access to version control.
  • Maybe the team does use version control, but they are are in a period where they are migrating to using a Salesforce DevOps product like AutoRABIT or Salto, and during that transition, they have to fallback to these type of comments.

One good thing about these comments is that they add an extra layer of business context that wouldn’t normally be there. The ticket where the requirement was defined probably has 10 times more context than what you could figure out by reading the code alone. Sometimes this context can make a huge difference, specially when troubleshooting something that was written years ago.

The problem though, is that if they are used everywhere, they can really clutter the code. They can also become outdated very quickly and start “telling lies”. It’s also not easy to tell which ticket corresponds to which part of the code, something that you could do if you used Git instead. Finally, they may reference tickets from old systems that are no longer available, so the ticket reference isn’t helpful anymore (notice in the sample above the first 2 entries reference a different tracking system).

Ultimately, using Git is the correct solution to this problem, but recognising that that is a decision that may not be in your control, my recommendation is that you use these type of comments with care; only use them when you think that the added business context outweighs the clutter that the comment creates and as a temporary measure while you work with your team to implement Git.

6.4 Aha! comments

Sometimes you are reading a piece of code that is really hard to understand. After reading for a bit, you may go “Aha! that’s what it does!”. Right there and then, add a comment explaining what you just figured out. Think what comment would’ve been useful 15 minutes ago when you started to decipher the code. That’s a great comment.

However, sometimes refactoring the code would be better than leaving a comment, as the comment is again prone to becoming out of sync with the code as soon as the requirements change. Whether to refactor or leave a comment depends on the complexity of the code, how much you understand it, whether there are good unit tests, etc. We’ll discuss refactoring in detail in the last chapter of the book.

6.5 Why comments

About 4 years ago (at the time of this writing), I wrote the code for HappySoup.io, a popular open-source library for discovering metadata dependencies in a Salesforce org. Some of the code for finding these dependencies and representing them in a tree-like structure is very complex. I remember I spent a few days with a pen and paper writing down the algorithm step by step until I finally cracked it.

When I finally wrote the code, I had to add comments explaining some of my design decisions. Here’s a snippet from the library, which is written in JavaScript

/**
 * This is the the ids of the returned dependencies, for which we want to query dependencies one level down
 * For example ClassA > Field1
 * Field1.Id is one of the ids that we want to check dependencies for
 * We don't necessarily want to check dependencies for every returned dependency.
 * 
 * This is because if a dependency has already been queried, we don't want to query it again and add its 
 * references under yet another node in the hierarchy/tree. This also prevents an infinite loop when classes or fields
 * have circular references (i.e ClassA > ClassB > ClassA ...infinity)
 */
let nextLevelIds = [];

dependencies.forEach(dep => {

    let alreadyQueried = (idsAlreadyQueried.indexOf(dep.id) != -1);

    /**
     * Whether the id has already been queried or not, we still want to show this node
     * on the tree, this allows circular references to be display at max one level down
     */
    result.push(dep);

    if(alreadyQueried){
        /**
         * if it's been queried already, we mark is as repeated and we dont add it to the list of ids
         * to query one level down
         */
        dep.repeated = true;
    }
    else{
        /**
         * if it's not been queried already, then we know it's safe to query it for dependencies
         * but only if it's not a dynamic reference
         */
        if(!utils.isDynamicReference(dep)){
            nextLevelIds.push(dep.id);
        }
    }

Those are some big comments, I admit. But I also admit, that if you asked me today why the code is doing what it is doing, I’d have no idea. I’ve been lucky that I’ve never encountered a bug in this part of the logic, but if I ever do, I feel reassured that these comments will help me remember why I wrote this the way I wrote it 4 years ago.

These types of comments, which explain the why behind certain design decisions—especially when influenced by non-obvious factors like performance, governor limits, or Apex-specific quirks—are some of the most valuable to have.

Likewise, there may be instances where when faced with a limitation of the Salesforce platform, you make a design decision that is outside the norm. It’s not a decision that you would normally make. In that scenario, a future developer may read the code and think “Why didn’t they do x,y,z instead?”.

This is a great opportunity for a “why not” comment. Leave a comment explaining why you didn’t take the normal path. This will help ensure future developers don’t remove that decision by thinking they know better.

A valid question is whether these large comments shouldn’t instead be formal documents in Notion, Confluence, Lucid Chart or something along those lines. External documentation, like comments, will quickly fall out of sync as the requirements change. In my experience, such documentation is useful for documenting high-level architectural changes, while comments, like the ones in the example above, are better for documenting implementation details.

6.7 Dead code

A very common (anti) pattern in Salesforce code bases is seeing Apex code that has been completely commented out. You may be reading an Apex class and suddenly you stumble upon something like this

**/***
@AuraEnabled(cacheable=true)
public static Account getSingleAccount() {
    return [
        SELECT Id, Name, Phone, Type
        FROM Account
        WITH USER_MODE
        LIMIT 1
    ];
}***/**

This is known as dead code. This code that is never called, not referenced anywhere, never executed, etc. It just sits there for years. No body dares to touch it or delete it because they fear someone may need it. I admit I have felt this fear before. Now, there are multiple reasons developers create dead code.

You missed the best part 😔. Join the community of 70+ paid subscribers who are embracing a software engineering mindset and benefiting from this exclusive content. Don't be left behind—stand out from the crowd!

Subscribe for exclusive Salesforce Engineering tips, expert DevOps content, and previews from my book 'Clean Apex Code' – by the creator of HappySoup.io!
fullstackdev@pro.com
Subscribe