Eliminate Cargo Cult Programming and Build a Stronger Team

My Auntie Pat Tern is one of those people who refuses to let family members wear cargo pants when we go out to dinner.  She says she doesn't want to participate in the 'cargo cult'.  She inspired me to look at how Cargo Cult Programming has affected our Salesforce org.

One of the best things about programming on the Salesforce platform is the wealth of resources online to help problem solve. It can be one of the worst things about programming on the platform as well if you aren't careful about the resources you choose to follow.  Some code samples use outdated practices, others may be snippets of code taken out of context, which may not behave as expected in a different context.

In one case, the Apex documentation team included System.assert statements in a code snippet.  Our new developer later copied the snippet into a utility class.  The developer found a great resource but didn't fully understand the difference between System.assert and System.debug.  The former builds in a fatal error, appropriate for test code but not for our essential platform automations. Sniffing out this cargo cult programming helped us know what additional training could help our developers.

A more frequently found cargo cult is the factory class to build basic objects. A factory class is still far better than using the very outdated SeeAllData in your test code.  But test data factory classes add unnecessary work for coders. Salesforce offers a newer and better technique with Test.loadData, which I wrote about in July.  It lets us shift the work of creating and maintaining sample data from the developer to the administrator.  

Other cargo cult programming that pops up in our Salesforce org relates to data sharing and security rules.  We teach our developers early on to understand the difference between code that runs only in Triggers vs code that may be run by end-users via UI customizations like pages, flows and actions. And we encourage our administrators to monitor org security in our code during code reviews and test executions.

Hunting out and eliminating cargo cult programming helps us build cohesion among the team that maintains our Salesforce platform and better understanding between administrators and developers who now work more closely together. This helps us spread the effort of org maintenance across a team and make use of clicks and code for our projects. End users benefit and our org is more reliable and easier to maintain as well.

What A Load Of Business Data

Auntie Pat Tern thinks Superman's x-ray vision is stupid because "how would he ever know where he's supposed to look and where he should not bother focusing?" That got me wondering about the data and tests in my Salesforce org.  Even clicks-not-code developers should have automated tests that validate configuration changes with existing and expected data. So what's the best way to know which data those automated tests should use?

Some Salesforce developers like to write automated tests with "SeeAllData=True", an outdated and bad practice. A test that can see all data is not the same as a test that is smart enough to see the data that needs testing. For example, a good test will test positive scenarios, negative scenarios and extremes. In other words:

• What if the data is exactly what we planned for -- records with all the right data in all the right places?
• What if the data falls outside of expected norms -- records with missing fields or invalid data, for example?
• What if the data is coming in from a data import operation or an integration and many records need to be processed?

Admins can follow a few simple steps to create automated tests to run against their configuration changes, and their efforts can be used by developers for better unit tests as well. Simply create sample data representative of the expected inputs. Then use code like the following to load that data in a unit test:


Note that you will want to use data that corresponds to the configuration changes that you are testing. In this example, configuration changes and business process automation around Account, Contact and Opportunity record creation will be tested. You can create a similar test class for other related objects, including custom objects.

This code can be run as a unit test by itself to validate configuration changes, or it can be called by other test code to set up data for more complex unit testing related to other code in the org.

Next week, I will look into some of the errors that might occur when you create CSV files of sample data and how to avoid those errors.

But for now, Auntie Pat Tern has a point, who wants Superman looking at just any old thing with his x-ray vision when he ought to be focusing on information that's most helpful.

SeeAllData = Fail

According to Auntie Pat Tern, it takes 21 days to break a bad habit, but Salesforce developers have had about that many API versions to break the bad habit of writing unit tests with "SeeAllData=true".

In most cases, this clause alone can render tests completely useless. Unit tests should prove that code functions according to specifications by asserting proper database manipulations--create, read, update, delete operations--as well as Visualforce navigation.

I looked at unit tests in my org and found this example of a test that failed to achieve its purpose:

This test should be asserting that the data for ordering samples can be created with just a contact and sample type defined.  Unfortunately the test relies on existing, live, org data rather than test 'silo' data because of "SeeAllData=true" in the first line. There are easy methods for unit tests to create their own test data without relying on live org data.

We encountered the following problems because of using "SeeAllData=true":

• It required us to maintain test data among our real data.
• When the test data was changed during data cleanup (in one case, Pat Tern's Account was deleted), the tests failed even though functionality was unchanged.
• Tests were not reliable between Sandboxes and Production orgs due to data differences rather than actual functionality.
• Apex Hammer Tests may not have been automatically monitored in our org for each Salesforce release since Hammer Tests are blind to live org data.

In the rare case where a specific piece of data may be required for your code to behave, consider using custom metadata types instead of Salesforce objects. Trailhead can help you learn more about how they allow you to move metadata and records between orgs and test functionality without needing to see all data in the org.

Pay Down That Technical Debt

Auntie Pat Tern says we should pay with cash, or at least pay our credit cards off every month because "debt," she says, "is like fresh fish, it seems great at first but gets old fast and then it really stinks."

It's Hammer Time!

So I looked at my Salesforce org to assess the level of technical debt we had accrued and to plan how we would start paying down that debt.

Sometimes technical debt comprises the shortcuts or mistakes that no one has time to correct when a project needs to be completed. Technical debt also develops over time.  As businesses mature and change, their technical solutions can fall behind and create technical debt.

A Salesforce org can be especially prone to this since Salesforce offers a wealth of new features for all orgs three times each year and not all companies make an effort to rewrite their technical solutions based on these new features. Luckily, Salesforce offers some tools to help us assess some of the technical debt associated with our code.

First, I checked our Hammer Test Status.  The data silo gauge revealed that we still had old (and sadly a few new) unit tests that were using "seealldata=true", which we needed to update.  It also revealed a couple of tests that were failing and so needed some attention.

Next I checked the API version on our Apex Classes.  Any class that is 10 or more versions behind the current API version needed a review, both on the code side and on the process side.

The third step for me was reviewing our org's documentation for outdated information on our code.

My org review also included configuration, managed packages and basic processes, as well.

We may not be able to pay down all of our technical debt right away, but a monthly review and reminder of how it accumulates will help us develop better in the future.  And as Auntie Pat Tern says, "It's not just pay me now or pay me later, you know.  It's pay me now or pay me later with additional compounded interest!"