How Long Should Apex Methods Be: Length, Depth, and Subtasks

The second piece of common wisdom when it comes to methods is that they should be short. How short they should be and the pros and cons of short methods is a larger discussion. In this chapter, we will see that it’s more complicated than simply saying “methods shouldn’t be longer than 15 lines”.

4.1 Short Methods

Generally speaking, methods should be short because short methods:

• Are easier to test in isolation
• Are easier to reason about
• Can be reused by other modules
• Isolate different levels of abstraction

Of course, the immediate question is: “But how short should they be?”

4.2 The Clean Code Philosophy

One school of thought is that methods should be as short as possible. The biggest proponent of this idea is Robert C. Martin, the author of the famous book Clean Code. In his book, Martin advocates for methods to be extremely small, to the point where they literally hold one block of code and nothing else.

Here’s an example from his book

private void includeSetupPages() {
  if (isSuite)
    includeSuiteSetupPage();
  includeSetupPage();
}

private void includeSuiteSetupPage() {
  include(SuiteResponder.SUITE_SETUP_NAME, "-setup");
}

private void includeSetupPage() {
  include("SetUp", "-setup");
}

private void includePageContent() {
  newPageContent.append(pageData.getContent());
}

private void includeTeardownPages() {
  includeTeardownPage();
  if (isSuite)
    includeSuiteTeardownPage();
}

private void includeTeardownPage() {
  include("TearDown", "-teardown");
}

private void includeSuiteTeardownPage() {
  include(SuiteResponder.SUITE_TEARDOWN_NAME, "-teardown");
}

As you can see, most methods are indeed one line long and the majority of them are pass-through methods. Pass-through methods are methods that simply pass their arguments to another method. Pass-through methods do offer certain benefits, which we will explore soon, but it is my opinion that making every method in your code a pass-through method is an extreme.

If we were to follow this pattern, our simple password generator from the previous chapter would go from this

public static String generatePassword() {
        String chars = '12345abcdefghijklmnopqrstuvwxyz';
        String password = '';
        while (password.length() < 8) {
            Integer i = Math.mod(
	                        Math.abs(Crypto.getRandomInteger()),
	                        chars.length()
	                      );
            password += chars.substring(i, i+1);
        }
        return password;
    }

To this

    public static String generatePassword() {

        String chars = getAllPossibleChars();
        String password = initialiseEmptyPassword();
        while (isPasswordLengthLessThanEight(password)) {
            password = appendRandomChar(password, chars);
        }
        return password;
    }

    private static String appendRandomChar(String password, String chars) {
        Integer i = getRandomIndex(chars);
        return password += chars.substring(i, i+1);
    }

    private static String getAllPossibleChars() {
        return '12345abcdefghijklmnopqrstuvwxyz';
    }

    private static String initialiseEmptyPassword() {
        return '';
    }

    private static Boolean isPasswordLengthLessThanEight(String password) {
        return password.length() < 8;
    }

    private static Integer getRandomIndex(String chars) {
        return Math.mod(
            Math.abs(Crypto.getRandomInteger()),
            chars.length()
        );
    }

    private static String appendRandomChar(String password, String chars, Integer i) {
        return password += chars.substring(i, i+1);
    }

What used to be a simple and elegant method of 11 lines, has become 36 lines long. I will agree that the main method, generatePassword , is now easier to read than the first version. For example, the string 12345abcdefghijklmnopqrstuvwxyz was a magic number (as discussed on section 2.12), and now the method getAllPossibleChars explains what this string represents. This is good. However, now there’s a lot more code to read, which means a lot more cognitive load. The “area” of this method has become so large that now we need to understand each individual method before we can really understand what’s going on.

Another side effect of this philosophy is that the containing class, Password, becomes much larger due to the all the small methods. This makes the class appear overly busy, leading to the suggestion that new functionality should be added in a separate class, such as PasswordMasker. If we had kept all the logic within the generatePassword method, a new requirement for masking passwords could simply be added as a new method, such as Password.mask(). Don't get me wrong; there is value in classes having a single responsibility. The Single Responsibility Principle is something we will explore in depth in a future chapter. However, my point here is that breaking down a method into many small methods can push us to create many additional classes. I argue that there’s also value in keeping related functionality within a single class as we will see when we talk about deep modules later in the chapter.

Finally, the biggest problem with this technique is that it makes us focus on the wrong thing. Methods can only be small if we split larger methods into smaller methods, so the real question is not how short methods should be but instead, when should we split methods into smaller methods. Does splitting a method increase its complexity, or does it decrease it? Are there situations when splitting a method into smaller ones has negative consequences? This is a much more interesting discussion than simply focusing on method length.

4.3 Very short methods

I mentioned earlier that there are benefits to very small methods, either pass-through methods or methods that immediately return something. Here are some scenarios where you should use very short methods.

4.3.1 To explain

Sometimes, you need to be able to explain why you made certain design decision, even if that design decision it just one line of code. Consider the following JavaScript code from my open-source project HappySoup.io.

/**
 * Some metadata types are not fully supported by the 
 * MetadataComponentDependency API
 * so we need to manually query related objects to find dependencies.
 * An example of this is the
 * EmailTemplate object, which is when queried, does not return 
 * WorkflowAlerts that reference the template
 */
function lacksDependencyApiSupport(entryPoint){
    return [
			    'Flow',
			    'EmailTemplate',
			    'CustomField',
			    'ApexClass',
			    'CustomObject'].includes(entryPoint.type);
}

This implementation is very simple. I simply check if a metadata type is part of the array. But the reason I’m doing that, and what it means to the overall flow, is not that simple. I needed to add a big comment to explain that these are the metadata types that are not supported by the MetadataComponentDependency Tooling API. If I didn’t have this method, I would have to use the array directly, and adding such a big comment in the middle of some other code would be highly disruptive. By moving the logic to a method, I suddenly have a lot more space to add a detailed comment.

4.3.2 To hide information

Another reason to create very small methods is to hide implementation details from the users of that code. This is typically known as encapsulation, or information hiding; this is very much related to modularity, which is a topic that will be discussed at length in a future chapter.

The idea is we don’t want to expose internal information regarding how something is implemented. A great example of this can be found on Mitch Spano’s Trigger Actions Framework. This is one of the popular open-source trigger frameworks that exist at the time of this writing. One of the functionalities this framework provides is the ability to bypass a specific trigger action at run time. In the MetadataTriggerHandler class, we can find the following method

/**
	 * @description Check if a Trigger Action is bypassed.
	 *
	 * @param actionName The name of the Trigger Action to check.
	 * @return True if the Trigger Action is bypassed, false otherwise.
	 */
	public static Boolean isBypassed(String actionName) {
		return MetadataTriggerHandler.bypassedActions.contains(actionName);
	}

To determine if a trigger action should be bypassed, the action name is added to a Set of strings called bypassedActions. Imagine if your code had to call MetadataTriggerHandler.bypassedActions.contains(actionName) every time it needed to check if an action needed to be bypassed. This would mean that your code has intimate knowledge of the internals of the MetadataTriggerHandler class.

By providing the isBypassed, the framework provides a simple API for callers to use. Callers don’t know to be concerned about how or what determines if an action should be bypassed.

4.3.3 To simplify the API

Another reason for using very short methods is when they help simplify the public API that you expose to your users. Here again we can look at an example from Mitch Spano’s Trigger Actions Framework.

/**
	 * @description Execute the Before Insert Trigger Actions.
	 *
	 * @param newList The list of new records being inserted.
	 */
	public void beforeInsert(List<SObject> newList) {
		this.executeActions(TriggerOperation.BEFORE_INSERT, newList, null);
	}

In the above snippet we see that the public method is a lot simpler than the method that it actually calls. Calling beforeInsert(newAccounts) is a lot simpler than calling this.executeActions(TriggerOperation.BEFORE_INSERT, newAccounts, null). If callers had to use the latter, they would need to know all the possible values of the TriggerOperation enum, and the fact that in this scenario, the last argument must be null. This is too much complexity for the callers to maintain. The public method makes all this complexity go away.

4.3.4 To give the action a name

In one of my implementations I had to concatenate two record IDs to form a single Id that was used to retrieve items from a map. The code that concatenated IDs was extremely simple

String uniqueKey = parent.Id+'-'+child.Id;

I had the same line of code at least 4 times in the same Apex class. Because it was such a simple line of code and I was certain I wasn’t going to change it any time soon, I was hesitant to create a method just for it. However, I noticed that the variable name wasn’t enough to give clarity on why I was doing this, so I wrapped the code inside of a method

String uniqueKey = createUniqueKeyForRelationship(parent.Id,child.Id)

The combination of the variable and the method, gave the concept a bit more room to explain itself. However, the method name was not enough, so I ended up adding a comment on the method signature as we saw on section 4.3.1. This is just another variation of the previous examples, but the focus here is that I wasn’t trying to simplify the API, have more space for a comment, or to hide internal implementation details. My goal was to give the action of concatenating two strings a name that could provide more context to the reader as to why I was doing that. This was a good reason to create a method even if the implementation is just one line of code.

These are all valid reasons for creating one-liner methods and you should not be afraid to use them. This doesn’t mean however, that all methods should be this short.

4.4 Longer methods

One good reason to have longer methods is to coordinate different internal methods into a single coherent functionality. This is a version of the Facade design pattern, where multiple moving parts are hidden behind a simpler API. Consider the getDependencies JavaScript method from HappySoup.io, which retrieves metadata dependencies using several Salesforce APIs. I’ve highlighted the calls to the internal methods for clarity.

async function getDependencies(){

  let query = **recursiveDependencyQuery**();

  await query.**exec**(entryPoint.id);

  let dependencies = query.**getResults**(); 

  dependencies = await **enhanceCustomFieldData**(dependencies);

  let unsupportedDependencies = await **createUnsupportedDependencies**(dependencies);
  dependencies.push(...unsupportedDependencies);

  try {
      let dependentPackages = await **getDependentPackages**(dependencies);
      dependencies.push(...dependentPackages);
  } catch (error) {
      console.log('error while getting dependent packages',error)
  }

  let files = **format**(entryPoint,dependencies,'deps');

  let csv = files.**csv**();
  let excel = files.**excel**();
  let packageXml = files.**xml**();
  let datatable = files.**datatable**();

  let dependencyTree = **createDependecyTree**(dependencies);
  let statsInfo = **stats**(dependencies);

  return{
      packageXml,
      dependencyTree,
      stats:statsInfo,
      entryPoint,
      csv,
      excel,
      datatable
  }        
}

The goal of this method is to coordinate several smaller methods in order to provide a coherent response to the client. If we were to follow a simplistic rule such as “methods shouldn’t be longer than 15 lines”, then I’d have to break this method down further. I encourage you to consider all the consequences that would happen if I broke this down into smaller methods that then call other smaller methods.

One advantage of methods that coordinate smaller methods is that they act as order of execution documentation. We can see all the moving parts required to achieve a particular goal. More importantly, longer methods can often provide depth, an attribute that is often overlooked.

4.5 Deep vs. Shallow Modules

To learn more about when it makes sense to split longer methods into smaller ones, we can draw inspiration from another author. In the book, A Philosophy of Software Design, author John Ousterhout introduces the idea of “deep” and “shallow” modules. Ousterhout makes the case that the best modules are those that are deep: they have a lot of functionality hidden behind a simple interface. Here, we shift the focus from method length to method depth.