Hardcoding

The final common mistake I want to discuss here is that of hardcoding within Apex—particularly, hardcoding any type of unique identifier such as an ID or a name. For IDs, this mistake is probably quite obvious for most developers as it is well established that between different environments, including sandbox and production environments, IDs can—and should—differ. If you are creating a sandbox from a production environment, then at the time of creation, the IDs are synchronized for any data that is copied down to the sandbox environment. Following this, IDs do not remain synchronized between the two and are generated when a record is created within that environment.

Despite this, many developers, particularly those working within a single environment such as consultants or in-house developers, will hardcode certain IDs if needed. For example, consider the following code:

for(Account acc : Trigger.new) {
    if(acc.OwnerId = 'SOME_USER_ID') {
        break;
    }
    //do something otherwise
}

This code is designed to skip updates on Account records within our trigger context owned by a particular user, most commonly an application programming interface (API) user or an integration user. This pattern enables a developer to filter these records out so that if an update is coming via an integration, actions are skipped and the integration can update records unimpeded.

Should this user ID change, then we will get an error or issue here, so it is wise to remove this hardcoded value. Given that the ID for the user should be copied from production to any sandboxes, you may ask why this is needed. Firstly, there is no guarantee that the user will not be changed for the integration, going forward. Secondly, for development purposes, when initially writing this code, the user will not likely exist in the production org, and so, in your first deployment, you will have to tweak the code to be environment specific. Thirdly, this also limits your ability to test the code effectively, going forward. We will see how shortly.

As an update to this code, some may recommend making the following change (note that this is precisely the instance where we would extract this query to a utility class; however, we are inlining the query here for ease of display and reading):

for(Account acc : Trigger.new) {
    User apiUser = [SELECT Id FROM User WHERE Username = '[email protected]'];
    if(acc.OwnerId == apiuser.Id) {
        break;
    }
    //do something otherwise
}

This code improves upon our previous code in that we are no longer hardcoding the user record ID, although we are still hardcoding the name. Again, should this change over time, then we would still have an issue, such as when we are working in a sandbox environment and the sandbox name is appended to the username. In this instance, the code would not be executable, including in a test context, without updating the record to be correct. This could be done manually every time a new sandbox is created or through the code in the test. This means that our test is now bound to this user, which is not a good practice for tests, should the user change again.

In this instance, we should remove the name string to a custom setting for flexibility and improved testability. If we were to define a custom setting that held the value (for example, Integration_Settings__c), then we could easily retrieve the custom setting and the username for a query at runtime without the query, as follows:

for(Account acc : Trigger.new) {
    Integration_Settings__c setting = Integration_Settings__c.
      getInstance();
    List<User> apiUsers = [SELECT Id FROM User WHERE Username = 
      :setting.Api_Username__c LIMIT 1];
    if(acc.OwnerId == apiUsers[0]?.Id) {
        break;
    }
    //do something otherwise
}

Such a pattern allows us many benefits, as follows:

• Firstly, we can now apply different settings across the org and profiles, should we so desire. This can be increasingly useful in orgs where multiple business units (BUs) operate.
• Secondly, for testing, we can create a user within our test data and assign their username to the setting for the test within Apex. This allows our tests to run independently of the actual org configuration and makes processes such as continuous integration (CI) simpler.
• We have utilized our safe navigation operator and retrieved a list of users (limiting to a size of one) to ensure that if no such user is found, we avoid any other exceptions such as our NullPointerException instance, as discussed previously.

There is, however, another fundamental fix we should make on our code—have you spotted it?

We should not be doing a query in a loop! Congratulations to the eagle-eyed readers who have been paying attention throughout and spotted the issue already. It is important when looking at some code that we do not overly focus on a single issue and fail to spot another one that may be far more troublesome. Our final improved code would, then, look like this:

Integration_Settings__c setting = Integration_Settings__c.
  getInstance();
List<User> apiUsers = [SELECT Id FROM User WHERE Username = :setting.Api_Username__c LIMIT 1];
for(Account acc : Trigger.new) {
    if(acc.OwnerId == apiUsers[0]?.Id) {
        break;
    }
    //do something otherwise
}

Finally, we could extract both of these lines (the retrieval of the custom setting and the user query) to a utility class to abstract away, for the entire transaction to use as needed.