Read more at jamessimone.net, or return to the homepage





Refactoring Tips & Tricks

Identifying areas for potential code reuse is not always obvious. People learn and remember things in different ways; on a large team, and in a large project, keeping the entirety of a code base in your head is frequently an improbable proposition. Some manage. Some find themselves rewriting functions monotonously, either on a tight sprint deadline or simply because it’s easier to copy paste than it is to generalize. Refactoring is, ultimately, a luxury — but one that is worthwhile to invest your time in, as you will reap the dividends of refactoring efforts the longer your code is in use.

I’d like to walk you through some of my favorite refactoring tools — both mental and tangible assets that may help you to identify repetitive code that it is safe to abstract upon or consolidate.

linkCollection Utilities

I have frequently touched upon the necessity of Apex List (and other iterable classes) helpers. In another programming language, you frequently have the option to extend the base library classes in order to add functionality as you see fit — since we don’t have that luxury in Apex, it instead makes sense to cover useful List extension methods, which can reside in a helper class. Once more, we’re left without the tools we might otherwise be armed by: since you can’t create static classes in Apex, I typically like to label my Utils classes as abstract. This prevents them from being erroneously instantiated at runtime.

The "two" biggest helper functions you will 100% find use for in your journey through Apex? Methods for creating Maps from Lists or Sets. I say "two" in quotes because you will also benefit from overloading these methods to give you greater flexibility (although you only need to adopt the methods you intend to use). Before showing them off, let's look at a little code that you might find familiar:

1link//a processing method in one of your classes

2linkprivate Map<Id, List<SObject>> getTasksKeyedToAccounts(List<Account> accounts) {

3link //you might prefer a lower allocation cost

4link //method to get the Ids for each account.

5link //that's legit! I also typically keep this

6link //method in the same utils class I'll be showing

7link Set<Id> accountIds = new Map<Id, Account>(accounts).keySet();

8link List<Task> tasks = [

9link SELECT Id, WhatId, etc ...

10link FROM Task

11link WHERE WhatId =: accountIds

12link ];

13link

14link Map<Id, List<SObject>> accountIdToTasks = new Map<Id, List<SObject>>();

15link for(Task t : tasks) {

16link if(accountIdToTasks.containsKey(t.WhatId)) {

17link accountIdToTasks.get(t.WhatId).add(t);

18link } else {

19link accountIdToTasks.put(t.WhatId, new List<SObject> { t });

20link }

21link }

22link

23link //do further processing higher in the call stack

24link return accountIdToTasks;

25link}

That's your garden-variety one-to-many example of code that is practically begging to be refactored. There is also the (much simpler) one-to-one example: the cases where you need to return a Map<Id, SObject>. In both instances, it's entirely possible that you will occasionally also have some additional filtering criteria when building your Map — it may be the case that at times you need to filter some of your SObjects out based on domain-specific business rules. Taking the previously shown example a bit further (and for more notes on making constants for your picklists, check out my post on Picklist Validation):

1link//back in getTasksKeyedToAccounts

2link//normally I would use previously constructed constants

3link//if these values, like the below, were coming from a picklist

4linkList<String> matchingTaskTypes = new List<String>{ 'Chat', 'Inbound' };

5linkfor(Task t : tasks) {

6link //if the task's type doesn't match some pre-approved list

7link //don't add it

8link if(matchingTaskTypes.contains(t.Type)) {

9link continue;

10link }

11link else if(accountIdToTasks.containsKey(t.WhatId)) {

12link accountIdToTasks.get(t.WhatId).add(t);

13link } else {

14link accountIdToTasks.put(t.WhatId, new List<SObject>{ t });

15link }

16link}

So ... let's review our use-cases:

Let's take a look at the tests, first:

1link@isTest

2linkprivate class CollectionUtilsTests {

3link @isTest

4link static void it_should_create_one_to_one_map_from_key() {

5link//this is an extremely dumbed down example

6link//you could accomplish the same thing with

7link//the standard Map constructor, but this method

8link//serves to show that an Id and a String can be used

9link//interchangeably in this context, and that the SObjectField

10link//token can be used to create the Map keys

11link List<Account> accounts = new List<Account>{

12link//I've shown the TestingUtils method enough times that I think

13link//we can skip the documentation on it. Google it or check

14link//my repo for the source code

15link new Account(Id = TestingUtils.generateId(Account.SObjectType))

16link };

17link

18link Map<String, SObject> expected = new Map<String, SObject> {

19link accounts[0].Id => accounts[0]

20link };

21link

22link System.assertEquals(expected, CollectionUtils.convertToMap(accounts, Account.Id));

23link }

24link

25link @isTest

26link static void it_should_create_one_to_many_map_from_key() {

27link Id accountId = TestingUtils.generateId(Account.SObjectType);

28link

29link List<Task> expected = new List<Task>{

30link new Task(WhatId = accountId, Id = TestingUtils.generateId(Task.SObjectType)),

31link new Task(WhatId = accountId, Id = TestingUtils.generateId(Task.SObjectType))

32link };

33link

34link Map<String, List<SObject>> actual = CollectionUtils.convertToListMap(expected, Task.WhatId);

35link System.assertEquals(expected, actual.values()[0]);

36link//yes, you can compare Ids and Strings directly without a cast

37link System.assertEquals(true, actual.containsKey(accountId));

38link//here you need a cast of some sort since next() returns Object

39link//clearly (String) would also suffice

40link System.assertEquals(accountId, (Id)actual.keySet().iterator().next());

41link }

42link}

This is the basic setup, using static convertToMap and convertToListMap methods, which I will show in a moment. Some things to note:

The more complex setup for each method that allows you to perform filtering operations (tests first!):

1link//in CollectionUtilsTests

2linkpublic class TestCollectionEvaluator extends CollectionUtils.CollectionEvaluator {

3link private final List<String> taskTypes;

4link

5link public TestCollectionEvaluator() {

6link super();

7link this.taskTypes = new List<String> {

8link 'Chat',

9link 'Inbound Call'

10link };

11link }

12link

13link public override Boolean matchesCriteria(SObject obj) {

14link//or you could do a "get" here

15link return this.taskTypes.contains(((Task)obj).Type);

16link }

17link}

18link

19link@isTest

20linkstatic void it_should_create_one_to_many_map_with_filter() {

21link Id accountId = TestingUtils.generateId(Account.SObjectType);

22link

23link List<Task> tasks = new List<Task>{

24link new Task(WhatId = accountId, Id = TestingUtils.generateId(Task.SObjectType)),

25link new Task(WhatId = accountId, Id = TestingUtils.generateId(Task.SObjectType)),

26link new Task(

27link WhatId = accountId,

28link Id = TestingUtils.generateId(Task.SObjectType),

29link Type = 'Chat' //again this would normally be a const

30link )

31link };

32link

33link Map<String, List<SObject>> actual = CollectionUtils.convertToListMap(

34link tasks,

35link Task.WhatId,

36link new TestCollectionEvaluator()

37link );

38link

39link System.assertEquals(tasks[2], actual.values()[0][0]);

40link}

41link

42link//etc, you get it - the next test is the one-to-one with filter

Now we've taken the original functionality and generalized it, adding an abstract class that only needs its children to implement a matchesCriteria boolean method to perform complex filtering. Let's take a look at the source code:

1link//since it can't be static

2linkpublic abstract class CollectionUtils {

3link public static Map<String, SObject> convertToMap(List<SObject> sObjectList, String field) {

4link return converttoMap(sobjectList, field, null);

5link }

6link

7link public static Map<String, SObject> convertToMap(List<SObject> sObjectList, SObjectField field) {

8link return convertToMap(sObjectList, field.getDescribe().getName(), null);

9link }

10link

11link public static Map<String, SObject> convertToMap(List<SObject> sobjectList,

12link SObjectField field, CollectionEvaluator eval) {

13link return convertToMap(sobjectList, field.getDescribe().getName(), eval);

14link }

15link

16link public static Map<String, SObject> convertToMap(List<SObject> sObjectList,

17link String fieldName, CollectionEvaluator eval) {

18link Map<String, SObject> mapping = new Map<String, SObject>();

19link for(SObject sObj : sObjectList) {

20link String key = (String)sObj.get(fieldName);

21link if(String.isBlank(key) ||

22link eval != null && eval.matchesCriteria(sObj) == false) {

23link continue;

24link }

25link mapping.put(key, sObj);

26link }

27link return mapping;

28link }

29link

30link public static Map<String, List<SObject>> convertToListMap(List<SObject> sObjectList, SObjectField idKey) {

31link return convertToListMap(sObjectList, idKey.getDescribe().getName(), null);

32link }

33link

34link public static Map<String, List<SObject>> convertToListMap(List<SObject> sObjectList, String idKey) {

35link return convertToListMap(sObjectList, idKey, null);

36link }

37link

38link public static Map<String, List<SObject>> convertToListMap(List<SObject> sObjectList, SObjectField idKey,

39link CollectionEvaluator eval) {

40link return convertToListMap(sObjectList, idKey.getDescribe().getName(), eval);

41link }

42link

43link public static Map<String, List<SObject>> convertToListMap(List<SObject> sObjectList, String idKey,

44link CollectionEvaluator eval) {

45link Map<String, List<SObject>> keyToListValues = new Map<String, List<SObject>>();

46link for(SObject sObj : sObjectList) {

47link String key = (String)sObj.get(idKey);

48link if(String.isBlank(key) ||

49link eval != null && eval.matchesCriteria(sObj) == false) {

50link continue;

51link }

52link else if(keyToListValues.containsKey(key)) {

53link keyToListValues.get(key).add(sObj);

54link } else {

55link keyToListValues.put(key, new List<SObject>{ sObj });

56link }

57link }

58link return keyToListValues;

59link }

60link

61link public abstract class CollectionEvaluator {

62link public abstract Boolean matchesCriteria(SObject obj);

63link }

64link}

That may look like a lot of boilerplate, but these methods are insanely useful. I count 10+ examples of the convertToListMap method being called in a random org, and 20+ calls to convertToMap. The base methods are 11 and 13 lines long, respectively; the full palette of them amounts to 56 lines of code. If you make use of these methods more than 5 times in your entire codebase, they're worth implementing, and your savings compounds as you make more and more use of them. In the org I just quoted, that's a savings of ~300 lines of code! Plus you can cut down lines if you intend to only use these with SObjects without complex fields; in that case, you only need the method definitions with the SObjectField tokens.

Measuring the performance of dev teams is more an art than a science, and developers have long pushed back on being measured by lines of code produced; in general, whenever LOC is the actual metric by which people are measured, copying and pasting is the preferred past-time. If the metric were which developers were removing the most code, things would be a lot more interesting: I'm not making the case that you should be using this as a performance metric, but removing code through generalizing key usage patterns saves you time and energy in the long run.

Since working with collections is such a frequent paradigm within Apex, think hard and observe often when interacting with and writing new code related to lists/sets/maps. Refactoring to an invocation you can consume in more places may take you a little bit up front, but will save you more and more as time goes on. I'm not trying to touch on an exhaustive list of CollectionUtils methods; rather, my point in this post (and in posts like Lazy Iterators is that because we work with collections so frequently, they are a ripe area of the codebase to achieve easy refactoring wins).

linkIdentifying Other Areas For Code Reuse

What are some other big areas where you can find refactoring potential in your Salesforce org? I've spoken about centralizing your SOQL methodology in the Repository Pattern; this doesn't just give you type-safety for all your Apex queries, it also allows you to test against your SOQL where clauses (in and of itself a good thing).

Code that focuses on callouts is also a good place to revisit. It can be verbose to always be instantiating HttpRequestMessage objects — there's an opportunity to consolidate things accordingly.

Those examples are the most generalized, but in each individual org there also exists different paradigms, complexities, and conformity to standards (both those that are widely available and self-imposed ones). With such creativity abounding, how can we best analyze our code in a time-efficient manner in order to identify low-hanging fruit?

linkCleaning Up Aura Components

One person I'd like to give a shoutout to is Justin Lyon, who is constantly active and helpful on the SFXD discord — on the subject of Aura components, he was the first I've seen to suggest generalizing your Aura actions to avoid a ton of the setCallback/getCallback boilerplate that comes with Apex/LDS interactions in Aura. He uses a static resource to ensure promisify can be imported via ltng-require tag (a problem obviated in LWC by the ability to create actual utils; for more info see the Readme in the linked Github for Justin's project):

1linkwindow.kit = (function Promisify(kit) {

2link var promisify = function (auraAction) {

3link return new Promise(

4link $A.getCallback(function (resolve, reject) {

5link auraAction.setCallback(this, function (res) {

6link var state = res.getState();

7link if (state === "SUCCESS") {

8link resolve(res);

9link } else {

10link reject(res);

11link }

12link });

13link

14link $A.enqueueAction(auraAction);

15link })

16link );

17link };

18link

19link kit.promisify = promisify;

20link

21link return kit;

22link})(window.kit || {});

Justin has a bunch of other utils that can get added to the global kit object, and they're all worth browsing through! He also had some helpful things to say on the potential downsides to using ltng-require, since scripts loaded via that means don't execute if the Aura component in question isn't rendered — Salesforce's backgroundUtilityItem component can be used as a cleaner means of accomplishing adding helper functions to Aura, and Justin has an example repo for that as well.


Me not having thought about the Aura action boilerplate just goes to show that switching context can sometimes lead to wearing the wrong hat; when I first started writing Aura components, (as I've talked about previously in my post on building a custom Lead Path LWC), I barely knew the difference between Aura and JavaScript — and by the time I did, the glaring code duplications in each of my components wasn't something I'd thought about. Think about the different contexts you switch into when writing code — is there an area for improvement beyond Apex?

linkEnter Sloppy

The (most excellently) named Sloppy is by far my favorite static code analysis tool — and because it's language agnostic, this means that you can use it to scan your entire codebase by filetype.

Sloppy works by tokenizing your code by block and analyzing tokens that are similar. This allows it to detect matching lines and lines that nearly match across files, and "rank" that code in terms of Sloppiness (where a higher score means more duplication) across your entire codebase. It's an extremely powerful tool — let's see it in action. (Note: sloppy only takes a few seconds to run, but if you want instant feedback on what's happening, I recommend running it on powershell if you're developing on Windows. In bash the output doesn't stream till the operation completes):

1link# in the directory you've downloaded sloppy to

2link./sloppy -ccls -o50 the/directory/your/code/resides

So let's see what's going on here — sloppy takes a few command line arguments to make your life easier. It automatically scans for a ton of commonly used file extensions by default:

The first argument, -ccls adds .cls to the list of file endings. The second arg, -o50 expands the printed list of most sloppy tokens to 50. You can tailor the number of results to suit your fancy; it defaults to 10. A third argument you might consider using would be -eTest (sloppy is case-sensitive with args); you may find that a lot of your duplication comes from test files, and might be OK with omitting that from the results to focus on the real issues. In tests, there is also something to be said for the occasional duplication, if it makes an individual unit test easier to read.

Many of the results are keyed to the specific business and are, ultimately, not interesting; I've run sloppy on this particular codebase many times, and I've winnowed down the particularly glaring repetitions already. An example of sloppy's output that is interesting would be the below snippet:

1link34 tokens & 3 skips (743 sloppiness, 0.32% of total) starting at:

2link=> C:\Users\myName\Documents\Code\someOrg\src\classes\Class1.cls:145

3link=> C:\Users\myName\Documents\Code\someOrg\src\classes\Class2.cls:100

1linkif (accountIdToContact.containsKey(acc.Id)) {

2link Contact con = accountIdToContact.get(acc.Id);

3link con.OwnerId = acc.OwnerId;

4link this .... //end token

Pretty benign repetition, but if there were many classes using that same owner assignment paradigm (and sloppy will print out all classes where the duplication occurs), I would definitely consider consolidating that logic. Interestingly, the biggest offender in this codebase is actually the Aura components with their repetitive action handling! Fool me once ...

Sloppy also outputs some cool stats regarding the overall "sloppiness" of your project in terms of duplication:

1linksummary:

2link

3linktotal tokens: 96206

4linktotal sloppiness: 234333

5linksloppiness / token ratio: 2.44

That last part — the "sloppiness / token ratio" means you should be looking for a lower score. The first time I ran sloppy on this codebase, the score was hovering around 12, if I recall correctly. I definitely put a lot of time into lowering that ratio!

linkClosing Thoughts On Refactoring

In the end, how you choose to refactor is your own perogative. That being said, I hope that for dealing with collections in Apex, as well as identifying other key repetitive areas in your codebase, this post will prove of service. At the very least, thinking about how your code may be bloated is always helpful in refining your craft.

As well, there are many static code analysis programs available. I happen to like sloppy because it's small, fast as hell, and it can be run on many different codebases. The gamified output also provides satisfying feedback as you refactor the code it identifies as being duplicated. Of course, I'll be curious to hear about the different tools other people use (I know some people swear by Apex PMD, but the analysis that Apex PMD provides is more in the sense of guidelines: don't perform SOQL in loops; don't create deeply nested if statements, etc ...) — share the knowledge 😀. Till next time!


Editkgeee34 wrote in to talk about ApexDoc; I've linked the version of the project that is being actively maintained, as the SFDC repo has been abandoned. It generates Markdown pages that self-document your Apex code — perfect for being hosted on a developer wiki site or even within your repo itself.


Postscript

For those following along, you'll note that for the first time in a while, there was a considerable delay in article publication times. Those delays may continue. My fiance and I adopted a puppy ... suffice it to say, my writing time is at an all-time low. On the other hand, we have an adorable little hound named Buckwheat romping around, and having him has been a blast so far:

The young Buck

Collection UtilitiesIdentifying Other Areas For Code ReuseCleaning Up Aura ComponentsEnter SloppyClosing Thoughts On Refactoring

Home Apex Logging Service Apex Object-Oriented Basics Batchable And Queueable Apex Building A Better Singleton Continuous Integration With SFDX Dependency Injection & Factory Pattern Enum Apex Class Gotchas Extendable Apis Future Methods, Callouts & Callbacks Idiomatic Salesforce Apex Introduction & Testing Philosophy Lazy Iterators Lightweight Trigger Handler LWC Composable Modal LWC Composable Pagination LWC Custom Lead Path Mocking DML React Versus Lightning Web Components Refactoring Tips & Tricks Repository Pattern setTimeout & Implementing Delays Sorting And Performance In Apex Test Driven Development Example Writing Performant Apex Tests



Read more tech articles