Refactoring conditionals to strategies (in .Net/C#)

Published on December 02, 2019

Tagged: #engineering

Follow me on twitter for more posts like this

I’m doing some work on a legacy code base and there are some common refactorings I do over and over. One of my favourite improvements is making long lists of conditionals easier to read and especially test.

I use the common refactor-to-strategies pattern from Martin Fowler to deal with these.

The original code and issues with it

Here is a simplified example of what the code typically looks like. There are heaps of problems with code like this. Especially when it’s just a tiny part of an enormous 10,000 line file!

I had a new junior engineer ask how best to tackle something like this. Here is what I told them.

  • Hungarian notation is hard to read, the prefix has no value in a strongly typed language, the actual type drifts from the original prefix over time. We should remove the prefixes.
  • Using Area and Area2 do not tell us what they are actually used for. Why is 2 different from the original one? We should name these properly if we can.
  • There is repetition in the sanitization. We should prevent this.
  • It’s difficult to test long conditionals. We should make it easier to test.
  • It’s difficult for a new dev to understand and modify this code. We should always make it easy to modify the code.
  • Because the conditionals are different, all of these might run but they all assign to the same variable. It doesn’t matter too much here because it’s just assignments but we should make it so that only one of these will run as a matter of good practice.
pubic partial class MovementMain: Page
{
  // ... a few thousand more lines of code
  if (strArea2.ToLower().Contains(" container"))
  {
    strMovementGroup = strArea2.Replace(" Container", "").Replace(" container", "");
  }
  else if (strArea.ToLower().Contains(" container"))
  {
    strMovementGroup = strArea.Replace(" Container", "").Replace(" container", "");
  }
  else if (intStatus == 3 && "Vehicle".Equals(strTransportType) && intRealQty == 0)
  {
    strMovementGroup = "Vehicle";
  }
  // ... a few thousand more lines of code
}

Refactored code and comments

I created an interface to describe the condition used to select a strategy and the specific strategy to use when selected.

The strategies are added to a list in the desired precedence order.

I then use linq to select the first applicable strategy and use its result.

There is more code here because of the interface and boilerplate around each class. But it’s much easier to understand and test (imho anyway!).

If a new developer needs to add a new condition here they just need to add a new condition to the list.

We don’t need to test if linq works. We have moved the business logic out of the aspx.cs page. If we did need to test the selector choosing we could move that to factory of some kind.

I also renamed all the variables so they make a little bit more sense.

I don’t show it here but the strMovementGroup value is later used in further conditionals. Now that we have a strategy selector we can also put the items that depend on this value in the strategy. The root conditional is the one that selects the strMovementGroup. This is really important for future refactoring.

I added an ‘empty’ strategy so we don’t have to worry about nulls.

pubic partial class MovementMain: Page
{
  // ... a few thousand more lines of code
  var movementGroupSelectors = new List<ICondition>()
  {
    new VehicleSelector(intStatus, strTransportType, intRealQty),
    new MovementSelector(strArea),
    new MovementSelector(strArea2),
    new UndefinedSelector()
  };

  var movementGroupSelector = movementGroupSelectors
    .First(selector =>
    selector.IsApplicable());

    strMovementGroup = movementGroupSelector.SanitizedName();
 // ... a few thousand more lines of code
}



  // The following would be in different files
public interface ICondition
{
    bool IsApplicable();
    string SanitizedName();
}

public class MovementSelector : ICondition
{
    private readonly string movementName;
    public MovementSelector(string movementName)
    {
        this.movementName = movementName;
    }
    public bool IsApplicable()
    {
        return movementName.ToLower().Contains(" container");
    }
    public string SanitizedName()
    {
        return movementName.Replace(" Container", "").Replace(" container", "");
    }
}

public class VehicleSelector : ICondition
{
    public static string VEHICLE_NAME = "Vehicle";
    public static int AVAILABLE_STATUS = 3;
    private readonly int activeStatus;
    private readonly string transportType;
    private readonly decimal realQuantity;
    public VehicleSelector(int activeStatus, string transportType, decimal realQuantity)
    {
        this.activeStatus = activeStatus;
        this.transportType = transportType;
        this.realQuantity = realQuantity;
    }
    public bool IsApplicable()
    {
        return activeStatus == AVAILABLE_STATUS && transportType.Equals(VEHICLE_NAME) && realQuantity == 0;
    }
    public string SanitizedName()
    {
        return VEHICLE_NAME;
    }
}

public class UndefinedSelector : ICondition
{
    public bool IsApplicable()
    {
        return true;
    }
    public string SanitizedName()
    {
        return string.Empty;
    }
}

Now sometimes this much refactoring will be overkill.

This happened to be an area where new conditions are likely. You have to use your own judgement to decide what is worth refactoring or not!

Darragh ORiordan

Hi! I'm Darragh ORiordan.

I live and work in Sydney, Australia building supporting happy teams that create high quality software for the web.

I also make tools for busy developers. Have you ever spent a week getting your dev environment just right?

My DevShell tooling will save you 30+ hours configuring your dev environment with all the best modern tools. Get it here

https://darraghoriordan.gumroad.com/l/devshell


Read more articles like this one...

List of article summaries

#developer-experience

How engineers can help deliver software effectively

Delivery managers and team leads have the responsibility to deliver a software system via an engineering team.

Your customer wants every feature to work perfectly and they want it delivered yesterday. Your team wants to learn and grow.

It’s a tough role managing all the stakeholders and creators in a project.

Engineers can help drive great delivery by empathising with and supporting the delivery manager or leads in a project team.

#engineering

Engineering systems for consistency and impact

Your most impactful engineering is done before you write any code.

It’s important to have some systems around how you approach problems to make sure you’re consistent every time.

These are some of the techniques I use to make sure I’m covering as many angles as possible when doing my pre-coding engineering.

#engineering

How software engineers can avoid commoditisation

Engineers spend most of their learning time on technical implementation content. Things like new frameworks, languages or cloud platforms.

But turning solutions into code is a tiny part of what you do and it’s getting less valuable year by year.

As we’ve seen with “no-code” and tools like GitHub Copilot, the implementation part of our role is increasingly becoming commoditised.

You could generalise that the value an engineer brings to a team is their ability to analyse problems and synthesise context. The part of your role as an engineer that will never be replaced by “no-code” or AI is this high level cognition.

The true human aspect of being an engineer is working in a team and considering other people’s ideas, emotions and thoughts while solving these problems.

So shouldn’t you train these meta-cognition skills as much as you train the specific technologies?

Every engineer should spend time learning and applying general tools for thinking. These tools are applicable to almost all problems so the compounded payback on your invested time is huge.

Clearer thinking will amplify all the other skills you have and any frameworks or tools you learn will give you results for the rest of your career.

Like any skill, improving the way you think takes deliberate study and practice.

These are some of the tools and systems for thinking that I refer back to all the time.

#engineering

20 questions for a valuable code review

I recently had an interesting discussion around the value of doing code reviews and the value of mandatory code reviews.

I think code reviews are extremely valuable and should be done by most organisations and teams.

A valuable code review will

  • pass institutional knowledge around the org
  • help all engineers grow their skills
  • maintain quality in the face of all the other time pressures your team faces

But how do you keep a code review valuable?