Fix this trigger… solved!

Thank you for everyone who participated in the Fix this trigger challenge!

I have much respect to anyone who at least attempted this challenge, regardless of whether or not you succeeded. Remember, resiliency is your #1 skill as a developer!

Here’s the original version that needs to be fixed:

// This lazy dude wrote code to auto-accept and close all cases!
trigger LazyEmployee on Case (after insert) {
  List<Case> newCases = new List<Case>();
  for (Case a : Trigger.new) {
    a.Status = "Closed";
    a.Owner  = "David Liu";
    newCases.add(a)
  }
  update newCases;
}

And here’s the fixed version:

// This lazy dude wrote code to auto-accept and close all cases!
trigger LazyEmployee on Case (before insert) {
  for (Case newCase : Trigger.new) {
    newCase.Status  = 'Closed';
    newCase.OwnerId = '005i0000000OAOO';
  }
}

This solution is what I consider to be the simplest working trigger. There are many different ways to write the same trigger with the exact same results, so don’t feel too bad even if yours looks drastically different than mine. Also, keep in mind that in most cases it’s better to write code that’s easy to understand, even if it’s not necessarily the most efficient.

All fixes made:

1. Switched from an “after” to a “before” trigger since we never reference the record’s ID.
2. Removed the use of another list. – this wasn’t necessary since we switched to a before trigger.
3. Changed the name of our loop variable from “a” to “newCase”. It’s very important to always use descriptive variable names!
4. Replaced all double quotes (“) with single quotes (‘). Good job to everyone who caught this!
5. Changed field from “Owner” to “OwnerId”. Note that even though the standard field is called Owner, the API name of the field is actually OwnerId!
6. Hard-coded the owner’s ID instead of the owner’s name. Lookup fields always use IDs since they are guaranteed to be unique. Hard-coding a user ID instead of querying for it is sometimes better since it’s quicker, user records are extremely stable, and you’ll have to hard-code some part of your SOQL query anyway if you go that route.
7. Removed the update DML statement. Additional DML statements aren’t necessary on the base records for before triggers

Frequently Asked Questions:

Why didn’t you do a SOQL query for the user ID instead of hard-coding it?

The is a legitimate question because hard-coding is generally bad – things have a habit of changing over time and breaking your hard-code! In this case however, it’s important to note that if you did a SOQL query, you’d still be hard-coding things!

Here’s a sample SOQL query you might use:
[SELECT Id FROM User WHERE FirstName = ‘David’ AND LastName = ‘Liu’];

Notice that using a SOQL query in the above is just as bad as hard-coding the ID instead because:
1. You’re hard-coding the first and last name
2. You might be returned more than one result if another David Liu joins the company (there are many at Google)!
3. A SOQL query takes longer for Salesforce to process than hard-coding the ID, so your Salesforce.com will be slower for users!

Could you have used a Custom Setting to set the Owner ID instead?

Yes, absolutely! Custom settings would make this trigger much more scalable! If the owner ID ever changed in the future, we could simply update our custom setting using point and click, instead of deploying a code change. A+ if you used a custom setting!

Hey. . . you know you could’ve used a workflow rule right?

I sometimes ask questions like these during interviews to make sure the dev has a well-rounded Salesforce understanding! We broke this cardinal rule just this one time, for the sake of learning =)

Why didn’t we first check to see if the values were correct before updating them?

The marginal benefit of checking to see if the values were already “Closed”, for example, aren’t worth the loss in readability. Also, there’s no harm in overriding them with the same value anyway!

Now if this trigger ran on update and we were doing more than just changing a few fields (ie creating new records instead), we’d definitely want to check the previous values of the fields first!

Should we have used Maps?

Not necessary for a simple trigger like this (no SOQL required trigger)! We certainly could have found a use case for them here, but no need introducing any more complexity than is needed!

Why didn’t you use the “one trigger per object” route instead?

I didn’t do this because I cover this topic in Chapter 8, which I haven’t written for you guys yet! Also, while this design pattern is awesome, it’s not totally necessary for smaller orgs that don’t have a lot of existing code running in them.

Hope you guys enjoyed this post – more challenges like these coming soon!
David