Posted: September 06, 2020

The XY Problem In Programming

The Problem Code

A question came up on the SFXD discord the other day about optimizing a section of code that was, apparently, leading to some CPU Timeout exceptions — everyone’s favorite Salesforce error. What better way to remind yourself of the multi-tenant nature of SFDC than by having your precious CPU cycles halted just so that the (paying) community as a whole continues to definitively have CPU cycles to burn through. Ah … take a deep breath in. Let it out. That’s capitalism, baby.

Where was I?

Oh yeah — let’s look at this “offensive” code (which I’ve cleaned up ever so slightly, as many of the assigments were done without spaces inserted):

//isDelete wasn't shown in the excerpt
//we as the reader don't know if it's passed in
//calculated, or what it really signifies
//oimList is initialized via SOQL, unshown
Decimal quantity = 0;
Map<Id,Decimal> oimProductIds= new Map<Id,Decimal>();
for(OrderInventoryMovement__c oim : oimList) {
  if(isDelete) {
    quantity = oim.Quantity__c * -1;
  }
  else {
    quantity = oim.Quantity__c;
  }
  if(oimProductIds.containsKey(oim.ProductId__c)) {
    Decimal quantityToAdd = oimProductIds.get(oim.ProductId__c);
    quantity = quantity + quantityToAdd;
    oimProductIds.put(oim.ProductId__c, quantity);
  }
  else {
    oimProductIds.put(oim.ProductId__c, quantity);
  }
}

List<Product2> poProdSetList= [SELECT Id, TotalQuantity__c FROM Product2 WHERE Id = :oimProductIds.keySet()];
for (Product2 poProd: poProdSetList) {
  quantity = oimProductIds.get(poProd.id);
  poProd.TotalQuantity__c = poProd.TotalQuantity__c + oimProductIds.get(poProd.id);
  //etc, add to list for further processing
}

There’s a code smell going on here — the multiple assignments to the quantity variable certainly could be cleaned up. Also, it’s possible that to the reader/maintainer of this code, it’s simply “obvious” that the assignments are mutually exclusive, but to the layman they certainly don’t read as such (what if it’s isDelete and the poimProductIds contains the ProductId__c Id already??). Still, best to not let the eyes glaze over too much; the question is one of efficiency, not whether or not the code stands up on its own.

There’s nothing really fishy going on here. Some SOQL is run, the list is iterated through, a quantity per ProductId__c is calculated 1 — then it’s appended to a custom field on the Product itself. And that’s what the feedback was — how could this piece of code possibly be contributing to CPU timeouts? Was the un-shown SOQL for fetching ridiculously unoptimized?

In asking these questions, the truth quickly came out: later on in the stack, a series of bizarrely unoptimized @future calls were being made and the synchronous DML following those operations were the real causes of the slowdown. Multiple assignments be damned — this code wasn’t actually even part of the problem at all!

The Solution

It hurts, sometimes, to look at code that we know could be refactored to read better — and you should clean up such code whenever you can. If the shown code came to me in code review, I would have some serious questions for the engineer submitting something that looked like that. When it’s extant in the codebase, things become slighly more nuanced.

The red herring nature of the shown code was helpfully summarized by linking to the XY problem — don’t try to fix Y if you know that X is the problem. Balancing things like this and the Boy Scout Rule requires judicial use of time and resources — here, the lesson is that the urge to scratch at the itch of this code was due to two things:

  • wanting to fix an eyesore (Y)
  • wanting to avoid refactoring the actual issue (X)

If you’re lucky, you have a robust suite of tests associated with your scary “Feature X” such that you can dive into fixing the mess without worrying about introducing regressions. If you aren’t … now might be a good time to start 😅.


Premature Optimization Wrap Up

Short post today. Watching the conversation unfold on Discord was a great example of why having a supportive community can help in something as seemingly innoccuous as root-cause analysis.

If you don’t follow me on LinkedIn, some small news to note — while I have been self-employed as a Salesforce developer for some time now, I recently made the move back into full time employment with Publicis Sapient. Looking forward to contributing within this organization!


  1. Something that has always bothered me in C# and Java languages is the lack of Operand types in the standard language. You can create them, of course, but it's a shame that the standard library doesn't give us tools to do this in a much more object-oriented way - if the mathematical operations acting on the quantity decimal could simply be passed to a generic function that applied the operands to the given decimal.

    ↩ go back from whence you came!

Written by James Simone, who spent the last year traveling the world. You should follow along on his adventures!

© 2019, a She & Jim production