When we have reached the limits of configuration in Salesforce, we don’t have to stop. We can continue extending the functionality with code; after all, code is what enables this platform to do unlimited things. Well written code is scalable and is able to grow and evolve as the business does, and it never becomes a roadblock. That being said, does code ever become a roadblock?
Yes, it often does. There are many systems where the legacy code is so intricate that it is difficult to update it without risk of breaking something unintentionally. When code is long and hard to read, the impact of updating it is often not easily known. So, how do you get around that?
How Code Becomes Bad
No one intentionally writes bad code, in fact most code starts out good. However, code is often always changing. As changes come in, depending on the time allocated, the resulting code can often resemble a patchwork quilt. For example, we need to do something with contacts, so we find a method that is already doing something with the contacts, and we decide to stick the new feature there. It is just a few lines – no harm done.
However, as more changes are added to the same code base, the methods and classes start getting longer and longer. The original method names are deceiving as more logic and features are added to the original methods, and now those methods are doing additional things. Then there comes a time when changes become difficult and it becomes unclear what a change might break. How can this be avoided?
In order to write maintainable code, there are a couple of basic rules that can be followed:
- Maintainable code should be small and modular
- Use intentionally revealing names for classes and methods makes the code more readable
Remember these two points and you’ve already made things easier. Let’s take a closer look at them.
Keep It Small and Unique
Breaking any big problem into small chunks always makes the problem easier to handle and manageable. The same goes for code.
The number one thing that can be done to keep the code manageable and future proof is to keep the methods and classes small. You shouldn’t have to scroll to read the whole method.
So, if there’s a method called updateContact; don’t make it so it also updates opportunities, and don’t make it so it also sends an email to the contact. One way to accomplish this is to ensure that a method is only doing one thing.
If a method is only doing one thing, and it has 100% test coverage, unless the business logic encapsulated in that method changes, the method never needs to change again. Think of them as small building blocks that are created and tucked away, and they will always work as intended.
Here is an example of a long method:
public void handleBeforeUpdate(List<Sale__C> newSales, Map<Id,Sale__C> oldMap){
// update audit fields
for(Sale__c s : newSales) {
if(oldMap != null){ // this is an update
Sale__c oldRec = oldMap.get(s.id);
if( oldRec.Audit_Completed__c == true && s.Audit_Completed__c == false) {
boolean canAudit = false;
Id currentProfileId= userinfo.getProfileId();
Profile curUserProfile = [Select Id,Name from Profile where Id = :currentProfileId LIMIT 1];
for(String pname: allowedProfileNames){
if(curUserProfile.name == pname){
canAudit = true;
}
}
if(!canAudit){
s.addError('Audit Box cannot be unchecked once checked');
}
}
else if(s.Audit_Completed__c == true){
s.Audit_Completed_Time__c = system.now();
s.Auditor__c = UserInfo.getUserId();
}
else { // an admin is unchecking the checkbox
system.debug('an admin is unchecking the sales audit checkbox');
s.Audit_Completed_Time__c = null;
s.Auditor__c = null;
}
} // insert - there is no oldmap
else if(s.Audit_Completed__c == true){
s.Audit_Completed_Time__c = system.now();
s.Auditor__c = UserInfo.getUserId();
}
}
for(Sale__c s : newSales) {
String category;
if(s.Amount__c < 1000){
s.Sale_Category__c = 'Small';
}
else if( s.Amount__c < 10000){
s.Sale_Category__c = 'Medium';
}
else if(s.Amount__c < 50000){
s.Sale_Category__c = 'Large';
}
else if(s.Amount__c > 50000){
s.Sale_Category__c = 'Extra Large';
}
}
}
It might be thorough, but this styling of code can get complicated, and it can get complicated fast. Here’s an example of how to make it more readable; do you see the difference?
public void handleBeforeUpdate(List<Sale__C> newSales, Map<Id,Sale__C> oldMap){
addErrorForUnauthorizedAuditChanges(newSales, oldMap);
handleAuditFieldChanges(newSales, oldMap);
setSaleCategory(newSales);
}
private void addErrorForUnauthorizedAuditChanges(List<Sale__C> newSales, Map<Id,Sale__C> oldMap){
// error code here
}
private void handleAuditFieldChanges(List<Sale__C> newSales, Map<Id,Sale__C> oldMap){
// audit field change code here
}
private void setSaleCategory(List<Sale__c> newSales){
// sale category code here
}
Keep Inner Workings of a Class Private
A lot of code exists where all methods are made public. That makes life very easy, especially when writing test classes for code coverage. All methods are simply called from the test classes to get code. But is that the right thing to do?
Consider this scenario: there is a class that validates an address. It calls three other methods that have some basic logic to ensure the validity of street, city, and postal code. These methods are public. This class is noticed by some other developers in the org who had a need to validate the postal code, so they decided to start using that method in their code.
public class AddressValidator{
public Boolean isAddressValid(String address){
return (isStreetValid() &&
isCityValid() &&
isPostalCodeValid)
}
public Boolean isStreetValid(){
// validation code
}
public Boolean isCityValid(){
// validation code
}
public Boolean isPostalCodeValid(){
// validation code
}
}
Later on, the business decided to pay for a service that more accurately validates the address. So, the three methods can be deleted and replaced by a call to a service. However, you can no longer delete the isPostalCodeValid() method because it is being used elsewhere. If these methods were made private to begin with, then they would not have been exposed and would not have caused code dependencies.
public class AddressValidator{
public Boolean isAddressValid(String address){
return PostalAddressValidationService.validateAddress();
}
}
So, only make public what needs to be seen from outside of the class being written. Making everything public creates unintended dependencies.
Any internal workings should be made private so that they can be changed as needed without breaking anything outside of the class. If you intend to write something that should be used by everyone, keep it in a separate class.
Use Meaningful and Intentionally Revealing Names
When meaningful and intentionally revealing names are used, the code does not need excessive commenting. The code also becomes easier to read and understand when you come back to it after a while.
Here are some examples:
Summary
In summary, to keep code maintainable, we should strive to keep the methods and classes small. This means that methods should only be doing one thing at a time, and method names should reflect what is actually happening in the method. Use public modifiers to reveal only what is necessary, and keep everything else private.
And lastly, anytime long messy code is encountered, time should be built in the estimates to first refactor the code. Let us all pledge to leave the code we touch better than we find it.
Resources
- Clean Code by Robert Martin
- The Pragmatic Programmer by Andy Hunt and Dave Thomas
- Code Complete by Steve McConnell
- Refactoring by Martin Fowler