Replace Conditional With Polymorphism
blogjoys-of-apex
Posted: June 06, 2021

Replace Conditional With Polymorphism

Table of Contents:

  • Our Example Application

  • Procedural Programming (Way Too Many Conditionals)

  • Refactoring To Use Polymorphism

    • A Brief Sojourn Into Further Polymorphism
    • Returning To The Overall Trip Satisfaction Concept

  • Replace Conditional With Polymorphism Closing Thoughts

I’d like to take you through a routine refactoring exercise to showcase the power that objects wield when used correctly. This is, in my opinion, the single most satisfying refactoring, because it showcases how maintanability improves by fixing an improper abstraction instead of letting it fester. This exercise will also highlight a curious junction between Test Driven Development (TDD) and Domain Driven Design (DDD) — which is to say that if the functionality that you’re working to refactor exists as one of the central concepts for the application you’re working on, it deserves to be represented properly as an object. If a subject matter expert with no coding experience whatsoever can look at the name of a class and say: “hey, I know what that is!” — you’re doing it right.

I spend a lot of time talking about Test Driven Development — which is to say, making use of the “red, green, refactor” mantra while making code changes. It might surprise you to learn that the first thing that we do is start making changes to the production-level code, but this is because the tests are already green. If we’re starting from the point of all tests passing, we have carte blanche to make updates as we see fit. Let’s dive in.

Our Example Application

The application I’ll be referring to over the course of this article was state of the art! … a few years ago, that is. The code has been maintained, but not touched much since the initial release, and there are the code equivalents of earmarks in a well-read book everywhere: this thing was rushed out the door. Our application takes in an enormous quantity of traffic information for major metropolitan areas, massages that data into a common format, and uses that information to determine things like road health, driver satisfaction, etc … as well as trying to do things like identify roads in need of maintenance based on overuse. While there are a few inner classes that somewhat represent a few of the metrics I’ve just mentioned, they are used inconsistently, and since they are inner classes, none of them are first-class citizens within the codebase.

For our example refactoring, we’ll be looking at just one of the metrics available to us — driver satisfaction. Our application measures driver satisfaction based on how often during an average trip a driver’s trip is interrupted:

  • if the driver encounters a traffic light but they don’t have to stop (green), their trip is considered (relatively) unaffected. On a long enough trip though, regardless of delay, satisfaction will decline.
  • if the driver encounters a traffic light and the warning light (yellow) is on, their trip begins to be affected negatively
  • if the driver encounters a traffic light and they have to sit and wait for it (red), their trip is negatively affected. There is also a split test being performed on some travelers that slightly downgrades the negative effects in the event that the wait is a small one (same for yellow).
  • if the driver encounters a stop sign - this ends up getting lumped into the warning category for yellow lights

Trips are stored in a custom object, Trip__c, and the various drive-related metrics are stored on the Trip object as well. All drives are recorded anonymously, so Trip has no concept of a specific driver, but we do store things like a trip’s duration. Trip has a related object, StreetTraveled__c, which contains one record related to trip per road used on any particular trip. There’s also some feature-flagging being accomplished through the use of Custom Metadata; in this case, TripEnvironment__mdt. No application is complete without lots of toggleable feature flags.

OK, enough overview — let’s dive on into the code.

Procedural Programming (Way Too Many Conditionals)

At the moment, code associated with the driver’s trip satisfaction are buried in a fairly large (1k+ LOC) class:

// in a much larger, outer class
private class TripSatisfaction {
  public final String roadName;
  public final Boolean isStop;
  public final String intersectionName;
  public final Decimal delayTime;

  public TripSatisfaction(String roadName, Boolean isStop, Decimal delayTime) {
    this.roadName = roadName;
    this.isStop = isStop;
    this.delayTime = delayTime;
  }

  public TripSatisfaction(String roadName, Boolean isStop, Decimal delayTime, String intersectionName) {
    this(roadName, isStop, delayTime);
    this.intersectionName = intersectionName;
  }
}

private static Map<String, List<TripSatisfaction>> getTripSatisfactionByTrafficColor(Trip__c trip, TripEnv__mdt tripEnvironment) {
  // code smell - magic strings being used as map keys
  Map<String, List<TripSatisfaction>> colorToTripSatisfaction = new Map<String, List<TripSatisfaction>> {
    'Red' => new List<TripSatisfaction>(),
    'Yellow' => new List<TripSatisfaction>(),
    'Green' => new List<TripSatisfaction>()
  };

  if (trip != null) { // code smell
    if (tripEnvironment == null) {
      tripEnvironment = new TripEnv__mdt(); // just another code smell
    }
    // also a code smell, but for a different reason
    List<StreetTraveled__c> tripStreets = trip.StreetsTraveled__r;

    for (StreetTraveled__c street : tripStreets) {
      TripSatisfaction satisfaction = street.IntersectionEncountered__c ?
        new TripSatisfaction(
          street.RoadName__c,
          street.StopSignEncountered__c,
          street.TotalDelayMinutes__c,
          street.IntersectionName__c
        ) :
        new TripSatisfaction(
          street.RoadName__c,
          street.StopSignEncountered__c,
          street.TotalDelayMinutes__c
        );

      if (tripEnvironment.AlternateSatisfactionMeasureEnabled__c && street.StopSignEncountered__c && street.TotalDelayMinutes__c < 1) {
        // some arbitrary requirement that's been added in after the fact
        // which complicates things, as new requirements always do
        colorToTripSatisfaction.get('Green').add(satisfaction);
      } else if (tripEnvironment.AlternateSatisfactionMeasurementEnabled__c &&
        street.StopLightEncountered__c && street.TotalDelayMinutes__c < 1) {
        colorToTripSatisfaction.get('Yellow').add(satisfaction);
      }
      // standard requirements follow
      else if (street.StopSignEncountered__c == false && street.StopLightEncountered__c == false) {
        colorToTripSatisfaction.get('Green').add(satisfaction);
      } else if ((street.StopLightEncountered__c && street.TotalDelayMinutes__c == 0) || street.StopSignEncountered__c) {
        colorToTripSatisfaction.get('Yellow').add(satisfaction);
      } else {
        colorToTripSatisfaction.get('Red').add(satisfaction);
      }
    }
  }

  return colorToTripSatisfaction;
}

I won’t get started — too much — on things that should jump off the page about this example snippet. It’s purposefully written to elicit groans, and those groans might come in many forms:

  • magic strings (red, yellow, green) instead of enums
  • complicated conditionals
  • map structure being returned with the magic strings
  • the decision to use (implicitly) a sub-query to return the List<StreetTraveled__c> children records. This falls over if you have more than 200 child records for a given Trip__c — looking at you, ride-share drivers!

Before creating too long a list of complaints, let’s see the calculations for overall trip satisfaction:

public void setTripSatisfactionMetric(List<Trip__c> trips) {
  for (Trip__c trip : trips) {
    TripEnv__mdt tripEnv = TripEnv__mdt.getInstance(trip.TripEnv__c);
    Map<String, List<TripSatisfaction>> colorToSatisfaction = getTripSatisfactionByTrafficColor(trip, tripEnv);

    Decimal tripSatisfaction = 100;

    List<TripSatisfaction> reds = colorToSatisfaction.get('Red');
    for (TripSatisfaction red : reds) {
      // seemingly arbitrary
      if (String.isBlank(red.intersectionName) && red.delayTime < 5) {
        tripSatisfaction -= 1.4;
      } else if (red.delayTime < 3) {
        tripSatisfaction -= 1.3;
      } else {
        tripSatisfaction -= 1.5;
      }
    }

    List<TripSatisfaction> yellows = colorToSatisfaction.get('Yellow');
    for (TripSatisfaction yellow : yellows) {
      if (String.isBlank(yellow.intersectionName) == false && yellow.delayTime > 1) {
        tripSatisfaction -= 1.2;
      } else {
        tripSatisfaction -= 1.1;
      }
    }

    List<TripSatisfaction> greens = colorToSatisfaction.get('Green');
    for (TripSatisfaction green : greens) {
      // some negligible amount that would only add up on an extremely long trip
      tripSatisfaction -= .2;
    }

    trip.TripSatisfaction__c = tripSatisfaction;
  }
}

Whew. With that excerpt out of the way, we can move on to our refactoring exercise, and discuss the rationale behind deciding to refactor this piece of code. Have you heard the phrase leaky abstraction before? The TripSatisfaction inner class is the perfect example of a leaky abstraction, because it only partially wraps the StreetTraveled__c child records for Trip__c; you can tell this is the case for two reasons:

  • a map structure is being used to categorize the data being returned
  • TripSatisfaction relates to the overall Trip__c.TripSatisfaction__c score without taking part in the calculation directly; this breaks encapsulation (I also show an example of how even within a class it’s possible to break encapsulation in Building An Apex Portfolio)

If you recall, trip satisfaction is one of the primary metrics our application measures, yet the calculations necessary to update a trip’s satisfaction score are buried within a monolithic class. Even worse, there is a significant amount of test setup necessary to drive out (excuse the pun) the ability to test trip satisfaction levels. We have a lot of tests that are performing integration-level work purely in order to work through all of the conditionals in our TripSatisfaction inner class — if anything, this is more evidence that the concept of TripSatisfaction deserves to be elevated within the codebase. Properly encapsulating the business concept of TripSatisfaction will reduce the complexity associated with test setup, improve the clarity of the production-level code, and make it easier to further support our application in the years to come by allowing us to more easily introduce further concepts related to the satisfaction of a trip without having to impact the other areas of the codebase that deal with this metric.

Refactoring To Use Polymorphism

When we look at the existing TripSatisfaction object, it’s a classic case of an improperly applied abstraction. If the so-called satisfaction object is being used to calculate trip satisfaction … it’s not really living up to its name. Furthermore, the inner class as it currently stands then gets passed around to several different places when callers need to access the portion of an overall trip satisfaction score as it relates to the red / yellow / green scoring paradigm. Identifying those two separate needs also leads us to a third that we’ll need to consider, in addition to these two:

  1. we need to centralize the logic for how a TripSatisfaction object gets created
  2. getting the overall satisfaction of a trip
  3. accessing the portion of a trip satisfaction score related to traffic light colors

Will help us to arrive at the ideal architecture for this object. Let’s begin!

public enum TrafficColor { RED, YELLOW , GREEN }

public abstract class TripSatisfaction {
  protected final Trip__c trip;
  protected final TripEnv__mdt env;

  protected Decimal overallSatisfaction;

  protected TripSatisfaction() {
    // more on this constructor, and the need for it
    // in a second
  }

  protected TripSatisfaction(Trip__c trip, TripEnv__mdt env) {
    this.trip = trip;
    this.env = env;
    this.overallSatisfaction = 100;
  }

  public abstract Decimal getOverallSatisfaction();
  public abstract Decimal getTrafficSatisfactionFor(TrafficColor color);
}

The prior version of TripSatisfaction has been discarded entirely as we promote this class (and the enum!) to its own file. We’re focusing, presently, on the API for the object we’d like to have; that includes an enum to formalize in a strongly-typed fashion the lower-level concept of a satisfaction score being comprised of three different sections. It also includes a zero-argument constructor — what’s up with that? Well, remember this section of the snippet above?

if (trip != null) { // code smell
  // etc ...
}

Null checks, and null pointer exceptions, are the bane of our existence as programmers. In Object Oriented Programming, however, there’s another (tangential) refactoring tool available to remove the hurdle of pervasive null checks: the Null Object pattern, which I briefly mentioned in the Object Oriented Basics post. Specifically, in our case, we will want a single Null Object version of TripSatisfaction, which will return an empty satisfaction score:

private class NullTripSatisfaction extends TripSatisfaction {
  private final Decimal sentinelValue = 0;
  public NullTripSatisfaction() {
    super();
  }

  public override Decimal getOverallSatisfaction() {
    return this.sentinelValue;
  }

  public override Decimal getTrafficSatisfactionFor(TrafficColor color) {
    return this.sentinelValue;
  }
}

Traditionally, Null Objects are implemented by means of a Singleton object, so as to not contribute meaningfully to overall heap size with tons of duplicated objects, which brings us to the method we’ll be using to drive out different kinds of TripSatisfaction objects:

// within TripSatisfaction.cls

private static final NullTripSatisfaction NULL_TRIP = new NullTripSatisfaction();

// ...

public static TripSatisfaction create(Trip__c trip, TripEnv__mdt env) {
  TripSatisfaction satisfaction;

  if (trip == null) {
    satisfaction = NULL_TRIP;
  }

  return satisfaction;
}

Now wait a moment! you might declare, as you look at the beginnings of what will end up being several conditionals. Aren’t you just moving the same conditional from where it was originally?? How is that object-oriented?? I thought we were getting rid of the conditionals! You’d be quite right to bring that up. However, you’ll note that the first step we identified was how these objects are constructed, and this is something that has been called out explicitly in Object Oriented textbooks since at least Clean Code came out (as you may recall me having explicitly quoting before in Extendable APIs):

The solution to this problem is to bury the switch statement in the basement of an ABSTRACT FACTORY, and never let anyone see it.

In our case, the “abstract factory” might currently “live” as a static method within TripSatisfaction. It doesn’t require a class of its own, at this current juncture; what it accomplishes, however, is the proper separation of concerns between how a trip’s satisfaction is measured versus how it is created.

Let’s move on to actually implementing a concrete version of TripSatisfaction:

// change from abstract to virtual; we are now working on the implementation

public virtual class TripSatisfaction {
  private final Trip__c trip;
  private final TripEnv__mdt env;
  private final Map<TrafficColor, Decimal> trafficColorToSatisfaction;

  protected Decimal overallSatisfaction;

  protected TripSatisfaction() {
    // left empty for NullSatisfaction object ...
  }

  private TripSatisfaction(Trip__c trip, TripEnv__mdt env) {
    this.trip = trip;
    this.env = env;
    this.trafficColorToSatisfaction = new Map<TrafficColor, Decimal>();
    this.overallSatisfaction = 100;
  }

  public virtual Decimal getOverallSatisfaction() {
    // TODO - implement the logic to update this number!!
    return this.overallSatisfaction;
  }
  public virtual Decimal getTrafficSatisfactionFor(TrafficColor color) {
    // this is now trivial to implement
    // and to update, if necessary
    return this.overallSatisfaction -
      this.trafficColorToSatisfaction.containsKey(color) ?
        this.trafficColorToSatisfaction.get(color) :
        0;
  }
}

With only a few keystrokes, we’ve already accomplished quite a bit. The only thing missing is pulling the conditional logic that affects a trip’s overallSatisfaction such that the corresponding TrafficColor map entries are populated, as well. When you have constructors set up which allow you to arrive at a number, or an object, as the result of some one-time calculations, I highly recommend keeping dependencies declared as final so that they are only set once and you have that guarantee of immutability:

public virtal class TripSatisfaction {
  private static final Decimal STARTING_SATISFACTION = 100;
  private static final Decimal GREEN_OFFSET = .2;
  // we no longer need to store the trip and env variables
  private final Map<TrafficColor, Decimal> trafficColorToSatisfaction;
  // now ALL the instance variables are final - awesome
  protected final Decimal overallSatisfaction;

  private TripSatisfaction(Trip__c trip, List<StreetTraveled__c> streetsTraveled, TripEnv__mdt env) {
    this.trafficColorToSatisfaction = new Map<TrafficColor, Decimal>{
      TrafficColor.RED => 0,
      TrafficColor.YELLOW => 0,
      TrafficColor.GREEN => 0
    };
    // you can declare a variable as final so long as it is set
    // only once (in the consturctor or when declaring it) -
    // even if you are setting it as the result of a function
    this.overallSatisfaction = this.calculateSatisfaction(trip, streetsTraveled, env);
  }

  public virtual Decimal getOverallSatisfaction() {
    return this.overallSatisfaction;
  }

  public virtual Decimal getTrafficSatisfactionFor(TrafficColor color) {
    // we now have the internal guarantee that all enums are accounted for
    return STARTING_SATISFACTION - this.trafficColorToSatisfaction.get(color);
  }

  private Decimal calculateSatisfaction(Trip__c trip, List<StreetTraveled__c> streetsTraveled, TripEnv__mdt env) {
    // everybody starts off perfectly happy!
    Decimal overallSatisfaction = STARTING_SATISFACTION;
    for (StreetTraveled__c street : streetsTraveled) {
      if (env.AlternateSatisfactionMeasureEnabled__c) {
        this.measureSplitTestSatisfaction(street);
      }

      this.measureStandardSatisfaction(street);
    }

    // now that the street-level counts have been tallied,
    // assign the scores once and for all
    for (Decimal offsetAmount : this.trafficColorToSatisfaction.values()) {
      overallSatisfaction -= offsetAmount;
    }

    return overallSatisfaction;
  }

  private void measureSplitTestSatisfaction(StreetTraveled__c street) {
    TrafficColor color;
    Decimal satisfactionOffset;
    if (street.StopSignEncountered__c && Street.TotalDelayMinutes__c < 1) {
      color = TrafficColor.GREEN;
      satisfactionOffset = GREEN_OFFSET;
    } else if (street.StopLightEncountered__c && street.TotalDelayMinutes__c < 1) {
      color = TrafficColor.YELLOW;
      satisfactionOffset = 1.1;
    }

    this.adjustColorSpecificScore(color, satisfactionOffset);
  }

  private void adjustColorSpecificScore(TrafficColor color, Decimal offset) {
    if (this.trafficLightToColor.containsKey(color)) {
      Decimal currentOffset = this.trafficLightToColor.get(color);
      this.trafficLightToColor.put(color, currentOffset + offset);
    }
  }

  private void measureStandardSatisfaction(StreetTraveled__c street) {
    TrafficColor color;
    Decimal satisfactionOffset;

    if (street.StopSignEncountered__c == false && street.StopLightEncountered__c == false) {
      color = TrafficColor.GREEN;
      satisfactionOffset = GREEN_OFFSET;
    } else if ((street.StopLightEncountered__c && street.TotalDelayMinutes__c == 0) ||
      street.StopSignEncountered__c) {
      color = TrafficColor.YELLOW;
      satisfactionOffset =
        String.isBlank(street.IntersectionName__c) == false &&
        street.TotalDelayMinutes__c > 1 ?
          1.2 :
          1.1;
    } else {
      color = TrafficColor.RED;
      if (String.isBlank(street.IntersectionName__c) && street.TotalDelayMinutes__c < 5) {
        satisfactionOffset = 1.4;
      } else if (street.TotalDelayMinutes__c < 3) {
        satisfactionOffset = 1.3;
      } else {
        satisfactionOffset = 1.5;
      }
    }

    this.adjustColorSpecificScore(color, satisfactionOffset);
  }
}

Et voilà!! There are a few things to call out, here:

  • There are more methods now — and more lines as a result of the closures — but each method has a clear purpose. The methods are named with an eye toward being as informative as possible. The math is only done in one place. There’s a bit of pesky duplication between our new measureSplitTestSatisfaction and measureStandardSatisfaction methods, as far as how the traffic colors and decimal offsets themselves get assigned, but there’s only so far one can go in keeping the code DRY before it starts to be overkill.
  • we’ve decoupled the code from SOQL dependencies by introducing the List<StreetTraveled__c> as a constructor parameter for TripSatisfaction. This kills two birds with one stone: it frees us from the need to perform DML in our unit tests, and it prevents the aforementioned issue with trips that have more than 200 streets traveled properly pulling all data back in our production-level code.
  • downstream consumers of TripSatisfaction no longer have to manually calculate the trip satisfaction per traffic color
  • you could continue the polymorphic work by reintroducing the old version of TripSatisfaction — which really was just wrapping the existing StreetTraveled__c object. This would clean up that “pesky” bit of duplication from bullet point one, at the expense of further heap allocations. I’ll show that off prior to returning to the current example:

A Brief Sojourn Into Further Polymorphism

Once again, note that we’re elevating this concept to a first-class citizen within the codebase. Does it have the best name? Perhaps not. Feel free to suggest a better one! For now, though, we’ll run with TripStreet:

public abstract class TripStreet {
  protected final StreetTraveled__c street;

  protected TripStreet() {
  }

  public TripStreet(StreetTraveled__c street) {
    this.street = street;
  }

  public abstract TrafficColor getColor();
  public abstract Decimal getScore();

  // for more on this, I recommend reading about the flyweight pattern
  private final static GreenTripStreet GREEN_SINGLETON = new GreenTripStreet();
  private final static SplitTestYellowTripStreet SPLIT_TEST_YELLOW_SINGLETON = new SplitTestYellowTripStreet();
  private final static RedTripStreet RED_SINGLETON = new RedTripStreet();
  private final static MiddleDelayRedTripStreet MIDDLE_RED_SINGLETON = new MiddleDelayRedTripStreet();
  private final static LowDelayRedTripStreet LOW_RED_SINGLETON = new LowDelayRedTripStreet();

  public static TripStreet create(StreetTraveled__c street, TripEnv__mdt env) {
    TripStreet returnStreet;
    if (env.AlternateSatisfactionMeasureEnabled__c &&
      street.StopSignEncountered__c
      && Street.TotalDelayMinutes__c < 1) {
      returnStreet = GREEN_SINGLETON;
    } else if (env.AlternateSatisfactionMeasureEnabled__c &&
      street.StopLightEncountered__c
      && street.TotalDelayMinutes__c < 1) {
      returnStreet = SPLIT_TEST_YELLOW_SINGLETON;
    } else if (street.StopSignEncountered__c == false && street.StopLightEncountered__c == false) {
      // this could be consolidated with the first if statement
      // but trying to keep nested conditionals to a minimum here
      // to help with rendering this page on mobile
      returnStreet = GREEN_SINGLETON;
    } else if (street.StopLightEncountered__c && street.TotalDelayMinutes__c == 0) ||
      street.StopSignEncountered__c) {
        // the only one that actually needs to perform additional logic on the
        // passed-in street
      returnStreet = new YellowStreetTrip(street);
    } else if (String.isBlank(street.IntersectionName__c) && street.TotalDelayMinutes__c < 5) {
      returnStreet = MIDDLE_RED_SINGLETON;
    } else if (street.TotalDelayMinutes__c < 3) {
      returnStreet = LOW_RED_SINGLETON;
    } else {
      returnStreet = RED_SINGLETON;
    }

    return returnStreet;
  }

  private class GreenTripStreet extends TripStreet {
    public GreenTripStreet() {
      super();
    }

    public override TrafficColor getColor() {
      return TrafficColor.GREEN;
    }

    public override Decimal getScore() {
      return GREEN_OFFSET;
    }
  }

  private class YellowStreetTrip extends TripStreet {
    public YellowStreetTrip(StreetTraveled__c street) {
      super(street);
    }

    public override TrafficColor getColor() {
      return TrafficColor.YELLOW;
    }

    public override Decimal getScore() {
      return String.isBlank(this.street.IntersectionName__c) == false &&
        this.street.TotalDelayMinutes__c > 1 ?
          1.2 :
          SPLIT_TEST_YELLOW_SINGLETON.getScore();
          // one of several ways you can avoid the 1.1 repetition
          // another would be to have YellowStreetTrip
          // extend from SplitTestYellowTripStreet
          // so that we could call super.getScore() here
          // but this would prevent us from using
          // SPLIT_TEST_YELLOW_SINGLETON (since YellowStreetTrip
          // requires calling super(street) in the constructor
    }
  }

  private virtual class SplitTestYellowTripStreet extends TripStreet {
    public SplitTestYellowTripStreet() {
      super();
    }

    public virtual override Decimal getScore() {
      return 1.1;
    }
  }

  private virtual class RedTripStreet extends TripStreet {
    public RedTripStreet() {
      super();
    }

    public override TrafficColor getColor() {
      return TrafficColor.RED;
    }

    public virtual override Decimal getScore() {
      return 1.5;
    }
  }

  private class LowDelayRedTripStreet extends RedTripStreet {
    public LowDelayRedTripStreet() {
      super();
    }

    public override Decimal getScore() {
      return 1.3;
    }
  }

  private class MiddleDelayRedTripStreet extends RedTripStreet {
    public MiddleDelayRedTripStreet() {
      super();
    }

    public override Decimal getScore() {
      return 1.4;
    }
  }
}

I know what you’re thinking — that’s a lot of inner classes! Notice how the granularity of what it’s possible to test is increasing with each of these iterations — now, we have access to what were previously only aggregated as the trip satisfaction offsets directly, and we can test those. We’re also stumbling into the Flyweight Pattern area with our factory methods for TripStreet and TripSatisfaction (especially with some of the returns being singletons), but I won’t spend time on that subject.

Crucially, we now only need a few variables between StreetTraveled__c and the TripEnv__mdt record to be truly setup in order to work through testing that we get the right TrafficColor and score back from whichever version of TripStreet is returned by the create() static factory method.

Take a look at what this potential re-re-factor (to coin a phrase) ends up looking like within TripSatisfaction:

// in TripSatsifaction.cls
// ...

private Decimal calculateSatisfaction(Trip__c trip, List<StreetTraveled__c> streetsTraveled, TripEnv__mdt env) {
  Decimal overallSatisfaction = STARTING_SATISFACTION; // 100
  List<TripStreet> tripStreets = new List<TripStreet>();

  for (StreetTraveled__c street : streetsTraveled) {
    tripStreets.add(TripStreet.create(street, env));
  }

  for (TripStreet tripStreet : tripStreets) {
    overallSatisfaction -= tripStreet.getScore();
    Decimal existingColorSatisfaction = this.colorToSatisfaction.get(tripStreet.getColor());
    this.colorToSatisfaction.put(tripStreet.getColor(), existingColorToSatisfaction + tripStreet.getScore());
  }

  return overallSatisfaction;
}

Now TripSatisfaction doesn’t even know how a trip’s “color” relates to its overall score; all it knows is that it can find the overall satisfaction number by doing some very simple math via the polymorphic TripStreet objects. Again — this particular area of the refactor might be overkill. I think it’s useful to highlight how, at a certain point, it’s "just objects all the way down." You can accomplish pretty much anything by adding indirection.

With that being said, we’ll return to the original refactoring to see how we ended up at the original call site for this code!

Returning To The Overall Trip Satisfaction Concept

Returning to our factory method (still within TripSatisfaction):

public static TripSatisfaction create(Trip__c trip, List<StreetTraveled__c> streetsTraveled, TripEnv__mdt env) {
  TripSatisfaction satisfaction;

  if (trip == null) {
    satisfaction = NULL_TRIP;
  } else {
    // one consolidated null check for env
    satisfaction = new TripSatisfaction(trip, streetsTraveled, env != null ? env : new TripEnv__mdt());
  }

  return satisfaction;
}

And now in our calling code:

public void setTripSatisfactionMetric(List<Trip__c> trips) {
  List<StreetTraveled__c> streets = [
    SELECT Id,
    StopLightEncountered__c,
    StopSignEncountered__c,
    ...
    FROM StreetTraveled__c
    WHERE Trip__c = :trips
  ];

  Map<Id, List<StreetTraveled__c>> tripToStreets = new Map<Id, List<StreetTraveled__c>>();
  for (StreetTraveled__c street : streets) {
    if (tripToStreets.containsKey(street.Trip__c)) {
      tripToStreets.get(street.Trip__c).add(street);
    } else {
      tripToStreets.put(street.Trip__c, new List<StreetTraveled__c>{ street });
    }
  }

  for (Trip__c trip : trips) {
    TripEnv__mdt env = TripEnv__mdt.getInstance(trip.TripEnv__c);
    TripSatisfaction satisfaction = TripSatisfaction.create(trip, tripToStreets.get(trip.Id), env);
    trip.TripSatisfaction__c = satisfaction.getOverallSatisfaction();
  }

  // etc ...
}

The calling code is now completely clean. It’s primarily concerned with getting the data in a sane, bulkified way, and passing that information to the TripSatisfaction object. We can cache the created TripSatisfaction objects to pass around as needed to other downstream callers; we can also create unit tests that provoke the extremely granular score / color conditions within our fully encapsulated class without needing to perform complicated setup.

It should be noted that the code isn’t perfect — and it’s not trying to be. Legitimate questions remain (and in fact are in part inspired by the new structure of the code):

  • is it possible for a trip to have no streets traveled? What should such a trip’s satisfaction score be?
  • other than sentinel values, nobody seems to have thought about what should happen if the overall satisfaction is less than 0! Should we handle that gracefully in the code?

Replace Conditional With Polymorphism Closing Thoughts

This is a really simple (and endlessly contrived) example. The polymorphic TripSatisfaction object is mostly abstracting out null checks that existed previously — the rest of this refactor is in support of that. It’s not meant to be perfect; it’s not meant to present the refactored code as “finished” by any means. It is meant to stimulate thought, and conversation. The second polymorphic refactor shown, with TripStreet, helps — but there’s always a fine balance to be struck in a codebase when it comes to the proliferation of objects, and perhaps TripStreet is taking the concept too far. Your mileage (😇) may vary.

What we’ve achieved is, again, partly a Domain Driven Design refactoring to promote TripSatisfaction to a first class citizen within the codebase. Doing so allows us to get rid of complicated tests involving many dependent objects, and replace them with tests that are more truly “unit”-based by removing SOQL and DML requirements (where possible). We’ve also shown how you can continue down the polymorphic path, increasing the specificity of your tests and decreasing coupling between objects (particularly at the database level). If you’re familiar with the Gang Of Four patterns, you’ll also note that both refactorings shown are examples of the Command pattern.

You should consider the “replace conditional with polymorphism” refactoring any time that you see multiple sets of conditionals acting on the same set of objects, particularly in the presence of loosely-typed objects like String-keyed Maps. Remember that this pattern is frequently paired with a Factory — be it a method that returns instances of your polymorphic object, or an actual class dedicated to that responsibility. For more reading on patterns, I would recommend checking out Creating A Round Robin Assignment, which walks through how to use the Visitor pattern, as well as Mocking Apex History Records, which shows off the DAO pattern. If you enjoyed this refactoring article, you might also enjoy Refactoring By Tidying.

That’s all for today! I hope you enjoyed this article — thanks, as always, for coming along on the Joys Of Apex journey with me! 👋

In the past three years, hundreds of thousands of you have come to read & enjoy the Joys Of Apex. Over that time period, I've remained staunchly opposed to advertising on the site, but I've made a Patreon account in the event that you'd like to show your support there. Know that the content here will always remain free. Thanks again for reading — see you next time!