[Test Refactoring Part 5] Dependency Simplification

311

Dearest Gentle Reader,

While today marks the end of our test refactoring journey, we still have much ground to cover.

To this point we have made great progress, resolving issues accompanied by sticky and infinite loops, pruning overgrown concrete dependency trees into topiaries that are the talk of the Ton, and uncovering the scandalous secrets of hidden dependencies that wreak havoc among the polite society of unit testers alike.

In today’s article we will hearken back to a Bridgerton-esque lifestyle that I find most appealing. Why you might ask? No, not just because I’ve binge watched the franchise from start to end yet again. Rather, Its simplicity.

By article’s end we will have completed the final steps to simplifying our codebase. This means that we will finally understand what it is to be free from coding complexity and be ready to sip tea with the Ton knowing all too well that sloths are being successfully launched into space on a thoroughly unit tested codebase.

Would you like that?

Remember to check out the Code Sloth Code Samples page for GitHub links to the repository of sample code!

Today’s article contains assumed knowledge about programming concepts such as classes, interfaces, polymorphism and mocking. Getting back into .Net? Check out our post on getting started with .Net 6 here to resume your .Net journey!

What a Mess

We’ve had many attempts at refactoring and unit testing the logic of our rocket launcher. Each of which has failed us. However, today we shall triumph!

using Microsoft.Extensions.Logging;
using Simplicity.Database;
using Simplicity.Infrastructure;
using Simplicity.WebApi;
using System.Text.Json;

namespace Simplicity.BusinessLogic
{
    public class RocketLaunchingLogic : IRocketLaunchingLogic
    {
        private IThrustCalculator thrustCalculator;
        private IRocketDatabaseRetriever rocketDatabaseRetriever;
        private IRocketQueuePoller rocketQueuePoller;
        private IRocketLaunchingService rocketLaunchingService;
        private ILogger<RocketLaunchingLogic> logger;
        private ICoordinateCalculator coordinateCalculator;
        private IAsyncDelay asyncDelay;

        public RocketLaunchingLogic(IThrustCalculator thrustCalculator, IRocketDatabaseRetriever rocketDatabaseRetriever, IRocketQueuePoller rocketQueuePoller, IRocketLaunchingService rocketLaunchingService, ILogger<RocketLaunchingLogic> logger, ICoordinateCalculator coordinateCalculator, IAsyncDelay asyncDelay)
        {
            this.thrustCalculator = thrustCalculator ?? throw new ArgumentNullException(nameof(thrustCalculator));
            this.rocketDatabaseRetriever = rocketDatabaseRetriever ?? throw new ArgumentNullException(nameof(rocketDatabaseRetriever));
            this.rocketQueuePoller = rocketQueuePoller ?? throw new ArgumentNullException(nameof(rocketQueuePoller));
            this.rocketLaunchingService = rocketLaunchingService ?? throw new ArgumentNullException(nameof(rocketLaunchingService));
            this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
            this.coordinateCalculator = coordinateCalculator ?? throw new ArgumentNullException(nameof(coordinateCalculator));
            this.asyncDelay = asyncDelay ?? throw new ArgumentNullException(nameof(asyncDelay));
        }

        public async Task TryToLaunchARocket()
        {
            var rocketLaunchMessage = await rocketQueuePoller.PollForRocketNeedingLaunch();
            if (rocketLaunchMessage == null)
            {
                await asyncDelay.DelayAsync(TimeSpan.FromSeconds(5));
                return;
            }

            try
            {
                var databaseResult = await rocketDatabaseRetriever.FindRocketStatistics(rocketLaunchMessage.RocketModelId);
                var requiredThrust = thrustCalculator.CalculateThrust(
                    databaseResult.ThrustValue1,
                    databaseResult.ThrustValue2,
                    databaseResult.ThrustValue3,
                    databaseResult.ThrustValue4,
                    databaseResult.ThrustValue5,
                    rocketLaunchMessage.numberOfSlothsToLaunch
                );

                var foodForJourney = await rocketDatabaseRetriever.GetFoodToFeedSlothsOnTheirJourney(rocketLaunchMessage.RocketModelId, rocketLaunchMessage.numberOfSlothsToLaunch);
                if (foodForJourney == null)
                {
                    if (rocketLaunchMessage.numberOfSlothsToLaunch < 10)
                    {
                        foodForJourney = new FoodForJourney { NumberOfCourses = 1, Foods = new[] { "only slightly toxic leaves" } };
                    }
                    else
                    {
                        throw new Exception("We can't cater a trip for 10 or more sloths without carefully planned food for space travel!");
                    }
                }

                var coordinatesToLandOnMoon = coordinateCalculator.CalculateCoordinatesToLand(rocketLaunchMessage.numberOfSlothsToLaunch, foodForJourney);

                var rocketLaunchResult = await rocketLaunchingService.LaunchRocket(rocketLaunchMessage.RocketModelId, rocketLaunchMessage.numberOfSlothsToLaunch, requiredThrust, foodForJourney);

                if (rocketLaunchResult.launchWasSuccessful)
                {
                    await rocketQueuePoller.RemoveMessageFromQueue(rocketLaunchMessage.messageId);
                }
                else
                {
                    throw new Exception($"Failed to launch rocket {rocketLaunchMessage.RocketModelId} with {rocketLaunchMessage.numberOfSlothsToLaunch} on board. Request id: {rocketLaunchMessage.messageId}. Retrying shortly.");
                }
            }
            catch (Exception ex)
            {
                logger.LogError($"Error caught while processing message {JsonSerializer.Serialize(rocketLaunchMessage)}. Error: {ex.Message} ");
            }
        }
    }
}

As you can see we last left our rocket launcher in a state of disarray. The poor constructor was simply bursting with parameters. But how can we resolve this?

The short answer is that there are more ways than we could possibly imagine. Each of which may come with a plethora of pros and equally so, cons.

One common theme of many approaches however, will likely be the introduction of abstraction. While we used the vertical lift and shift to help us prune deep dependency trees, did you know that it can be used in another way?

Sloth Side Quest: The Lateral Lift and Shift

Let’s take a quick recap of the vertical lift and shift. As demonstrated below we can use this strategy to trim dependency trees at the point of interface creation. In this example, we can see that we are generating only a single interface.

Did you know that you can perform a lift and shift with multiple (lateral) interfaces at a time? This technique is crucial to the simplification of bloated constructors.

Unlike the vertical lift and shift, the lateral lift and shift may produce one or more interfaces of cohesive functionality, each of which are aggregated by the original class. These interfaces may expose the same methods of their consolidated types, or combine them via new methods altogether.

Cohesive Interfaces… Eh?

If the lateral lift and shift involves creating interfaces of cohesive functionality, what exactly does cohesive mean?

This is where software design becomes beautifully subjective and fraught with debate. Let’s take a look at our existing conundrum again:

private IThrustCalculator thrustCalculator;
private IRocketDatabaseRetriever rocketDatabaseRetriever;
private IRocketQueuePoller rocketQueuePoller;
private IRocketLaunchingService rocketLaunchingService;
private ILogger<RocketLaunchingLogic> logger;
private ICoordinateCalculator coordinateCalculator;
private IAsyncDelay asyncDelay;

We have dependencies that:

• Calculate thrust
• Retrieve rocket information from a database
• Poll a queue for rocket launch signal
• Launch a rocket
• Log
• Calculate coordinates
• Async Delay

By definition, a cohesive interface (be it a literal .Net interface definition, or simply the methods expressed by a concrete class) contains methods of highly related functionality. The obvious choice for a cohesive interface from our existing list might therefore be:

public interface ITripCalculator
{
  Calculate thrust
  Calculate coordinates
}

This is because the IThrustCalculator and ICoordinateCalculator both:

• Functionally: perform “calculations” – it’s literally in their names
• Logically: help to safely navigate a rocket ship and its passengers to its desired destination

But hang on, doesn’t the IRocketLaunchingService also help with logically getting sloths to the moon? Yes, it absolutely does. So while ITripCalculator makes a cohesive interface, we could also equally create:

public interface IRocketNavigationSystem
{
  Calculate thrust
  Calculate coordinates
  Launch rocket
}

As you can see, just by looking at the dependencies of a bloated class we can draw circles around different groupings that could create a cohesive interface. It may be based on the type of functionality that they perform or the relatedness of their business logic.

While some may be more obvious than others, the subjectivity and overwhelming possibilities are bound to spark some interesting debate.

Cohesion Through the Lens of Simplification

Another way to rationalise interfaces can be to look at the logical flow of the existing code. If we look at TryToLaunchARocket in the first code sample, we can see that it:

• Polls a queue for a trigger message to launch a rocket, terminating early if none exists
• Finds rocket statistics from the database
• Calculates thrust based off this data
• Finds food to serve the sloths based off trigger message, handling some logic if we cannot find food, possibly throwing exception
• Calculates coordinates for # of sloths and their menu
• Launches rocket and performs some queue logic based off the launch status

We could potentially group this related functionality together:

The above screenshot is a visual representation of this type of approach. Green braces represent the retained logic and red represents the logic which could be laterally lifted and shifted. In summary, green code represents rocket orchestration logic, while red code represents the nitty gritty of how to launch a rocket; more on this below.

This approach abstracts 4 of our 7 dependencies into 1, resulting in a constructor that would depend only on 4 inputs. Furthermore this extracted code could have a vertical lift and shift applied to the logic of food planning. This would ensure that the extracted logic itself wasn’t unnecessarily complicated at unit testing time.

As we can now see, the ways in which this code could be made more unit testable by slicing and dicing the existing functionality is nearly endless.

I’m sure some readers might even be so bold as to suggest we implement it entirely with CQRS!

Programming is Both a Practice of Engineering and Art

With so many unique options available how on earth does one proceed, then?

With pragmatism. Identify specific problems that you are trying to solve and implement a solution that helps to mitigate them. If you can justify the why of your decision, you’re engineering better than most!

You’re almost guaranteed to find a critic who will raise questions during code review regardless of how appropriate the solution is, what methodology/design pattern/trend was implemented, or how many hours you spent laboring over it. At times these criticisms will relate to your own subjective artistic expression, be it a variable name, or composition of classes.

This is why is is imperative to engineer with intention! Stand steadfast alongside your decisions and substantiate why they were made. However in doing so, keep an open ear to alternative suggestions that may enhance your own ideas, or replace them entirely.

Writing code with the express goal of implementing a design pattern or adhering to purist dogma without a logical reason as to why will inevitably cause more harm than good. Start with sloth code and iterate on complexity/design/optimisation as objective requirements dictate.

The Subjective (Yet Substantiated) Opinions of this Code Sloth

My goal is to reduce the complexity of unit testing this class. This will be aided in part by a simplification of its constructor and the number of dependencies that must be orchestrated to write each test case. Unit testing will be most substantially aided however by simplifying the business logic of the class, as this will reduce the number of logical flows that our tests will have to navigate within it.

Therefore, performing a lateral lift and shift by abstracting cohesively behind a single method on an interface as demonstrated in the screenshot above will drastically help with this goal.

One red-flag that helped to influence this decision was the early return statement of the no-launch-message flow. It would likely complicate the solution if we needed to lift and shift a return statement. This is because the consumer of the new code would also have to perform a similar logical check and return early to ensure the short-circuit was retained. This is not to say it shouldn’t be done, rather, in this case the effort of additional refactoring to keep the code simple would likely have a diminishing return on value-add.

With this approach in mind, we can now revisit the intentions of our classes:

• RocketLauncher.cs (as it is currently named) is responsible for the lifetime of the application; the difficult to test loop driven by a cancellation token
• RocketLaunchingLogic.cs (as it is currently named) is responsible for identifying if we have a signal to launch a rocket, requests a launch if so and handles the cleanup of a successful launch
• yet-to-be-created-class will somehow take care of launching the rocket for us and will return an indication of whether the endeavor was sucessful or not

Let’s start by doing some tidy up.

Firstly, we rename RocketLauncher.cs and its class to RocketLaunchingBackgroundService to better reflect its intention. BackgroundService is a well-known .Net term and implies that this class is responsible for lifetime management.

public class RocketLaunchingBackgroundService : BackgroundService

Then, let’s rename RocketLaunchingLogic.cs to RocketLaunchingCoordinator to indicate its new responsibility, along with its interface. Some people may scoff at using the suffix Coordinator, however, I find it to be an appropriate description of the responsibility of the class – do rocket launches not need to be carefully coordinated, after all?

public class RocketLaunchingCoordinator : IRocketLaunchingCoordinator

The Technique of Lateral Lift and Shift

Finally lets laterally lift and shift the red section of the class into a new class called RocketLauncher. The easiest way to do a lateral lift and shift (behind a consolidated single method) is to setup your foundation first:

• Create a new empty class
• Add a parameter-less public void method
• Copy your desired code directly into it

Then, work backwards to address all of the red Visual Studio squiggles, by fixing:

• Unknown member variable errors by bringing across the required dependencies and generating a constructor
• Missing method-scoped variables by adding a parameter(s) to the method. In this case it will allow the rocket launching message to continue to be consumed within the new function
• The return type of the method to satisfy our consumer (if we aren’t also refactoring that). In our case we can add an async return type that matches the exact same result of our rocket launch call, as this will allow the caller to continue to work without additional refactoring

Finishing these steps produces our new class:

using Simplicity.Database;
using Simplicity.Infrastructure;
using Simplicity.WebApi;

namespace Simplicity.BusinessLogic
{
    public class RocketLauncher : IRocketLauncher
    {
        private IThrustCalculator thrustCalculator;
        private IRocketDatabaseRetriever rocketDatabaseRetriever;
        private IRocketLaunchingService rocketLaunchingService;
        private ICoordinateCalculator coordinateCalculator;

        public RocketLauncher(IThrustCalculator thrustCalculator, IRocketDatabaseRetriever rocketDatabaseRetriever, IRocketLaunchingService rocketLaunchingService, ICoordinateCalculator coordinateCalculator)
        {
            this.thrustCalculator = thrustCalculator ?? throw new ArgumentNullException(nameof(thrustCalculator));
            this.rocketDatabaseRetriever = rocketDatabaseRetriever ?? throw new ArgumentNullException(nameof(rocketDatabaseRetriever));
            this.rocketLaunchingService = rocketLaunchingService ?? throw new ArgumentNullException(nameof(rocketLaunchingService));
            this.coordinateCalculator = coordinateCalculator ?? throw new ArgumentNullException(nameof(coordinateCalculator));
        }

        public async Task<RocketLaunchResult> LaunchARocket(RocketLaunchMessage rocketLaunchMessage)
        {
            var databaseResult = await rocketDatabaseRetriever.FindRocketStatistics(rocketLaunchMessage.RocketModelId);
            var requiredThrust = thrustCalculator.CalculateThrust(
                databaseResult.ThrustValue1,
                databaseResult.ThrustValue2,
                databaseResult.ThrustValue3,
                databaseResult.ThrustValue4,
                databaseResult.ThrustValue5,
                rocketLaunchMessage.numberOfSlothsToLaunch
            );

            var foodForJourney = await rocketDatabaseRetriever.GetFoodToFeedSlothsOnTheirJourney(rocketLaunchMessage.RocketModelId, rocketLaunchMessage.numberOfSlothsToLaunch);
            if (foodForJourney == null)
            {
                if (rocketLaunchMessage.numberOfSlothsToLaunch < 10)
                {
                    foodForJourney = new FoodForJourney { NumberOfCourses = 1, Foods = new[] { "only slightly toxic leaves" } };
                }
                else
                {
                    throw new Exception("We can't cater a trip for 10 or more sloths without carefully planned food for space travel!");
                }
            }

            var coordinatesToLandOnMoon = coordinateCalculator.CalculateCoordinatesToLand(rocketLaunchMessage.numberOfSlothsToLaunch, foodForJourney);

            var rocketLaunchResult = await rocketLaunchingService.LaunchRocket(rocketLaunchMessage.RocketModelId, rocketLaunchMessage.numberOfSlothsToLaunch, requiredThrust, foodForJourney);
            return rocketLaunchResult;
        }
    }
}

With our new found clarity in the logic of launching rockets, can you spot the error? Amongst the bloated complexity of our existing rocket launching logic, would you believe that our code has forgotten to provide the coordinates that the rocket should land on the moon! Where have the rocket ships been landing all of this time?!

When classes are SOLID (specifically in this case the class has achieved a simpler single responsibility), they almost go out of their way to tell you about the problems that we have introduced into them. While we can’t do much about the sloths that may or may not be floating aimlessly about in space at the moment, we’ll fix this omission in the final solution’s code to make sure it doesn’t happen again.

After extracting an interface from our RocketLauncher, adding it as a dependency to our Coordinator and re-generating the constructor, we are left with a much simpler RocketLaunchingCoordinator!

using Microsoft.Extensions.Logging;
using Simplicity.Infrastructure;
using System.Text.Json;

namespace Simplicity.BusinessLogic
{
    public class RocketLaunchingCoordinator : IRocketLaunchingCoordinator
    {
        private IRocketLauncher rocketLauncher;
        private IRocketQueuePoller rocketQueuePoller;
        private ILogger<RocketLaunchingCoordinator> logger;
        private IAsyncDelay asyncDelay;

        public RocketLaunchingCoordinator(IRocketLauncher rocketLauncher, IRocketQueuePoller rocketQueuePoller, ILogger<RocketLaunchingCoordinator> logger, IAsyncDelay asyncDelay)
        {
            this.rocketLauncher = rocketLauncher ?? throw new ArgumentNullException(nameof(rocketLauncher));
            this.rocketQueuePoller = rocketQueuePoller ?? throw new ArgumentNullException(nameof(rocketQueuePoller));
            this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
            this.asyncDelay = asyncDelay ?? throw new ArgumentNullException(nameof(asyncDelay));
        }

        public async Task TryToLaunchARocket()
        {
            var rocketLaunchMessage = await rocketQueuePoller.PollForRocketNeedingLaunch();
            if (rocketLaunchMessage == null)
            {
                await asyncDelay.DelayAsync(TimeSpan.FromSeconds(5));
                return;
            }

            try
            {
                var rocketLaunchResult = await rocketLauncher.LaunchARocket(rocketLaunchMessage);
                if (rocketLaunchResult.launchWasSuccessful)
                {
                    await rocketQueuePoller.RemoveMessageFromQueue(rocketLaunchMessage.messageId);
                }
                else
                {
                    throw new Exception($"Failed to launch rocket {rocketLaunchMessage.RocketModelId} with {rocketLaunchMessage.numberOfSlothsToLaunch} on board. Request id: {rocketLaunchMessage.messageId}. Retrying shortly.");
                }
            }
            catch (Exception ex)
            {
                logger.LogError($"Error caught while processing message {JsonSerializer.Serialize(rocketLaunchMessage)}. Error: {ex.Message} ");
            }
        }
    }
}

As you can see, much like the lift and shift and vertical lift and shift, the lateral lift and shift does not have to require a substantial re-write of your codebase to achieve its benefits!

We now have a simpler constructor, which means simpler test setup and mocking at unit testing time. We also have less business logic and can focus on the new single responsibility of our class – coordinating rocket launches!

By abstracting the logic of launching the rocket behind a single method on an interface, we can easily mock the different results that it could possibly return. We no longer need to worry about mocking every single part of launching a rocket to see how the coordinator would use a given result.

Where Does It End?

In today’s article we performed one iteration of lifting and shifting. But is that enough? We could literally continue to iterate our refactoring ad nauseum.

One may argue that our new RocketLauncher has too many dependencies, or too much logic. Should it, for example, be responsible for knowing the intimate details of food preparation, if the class is only responsible for orchestrating the complexity of the launch itself?

Continue to step back from your refactored code and ask these questions. Then proceed with further lifting and shifting as required until your solution fits your desired shape. The code of the final Simplicity project will contain additional refactoring using this approach.

Non Functional Code

Before we conclude our series we have one final concern to address: non functional code.

This code, while still being functional does not align with functional principals. While having absolutely no state within a class at any time is (in my opinion) a nightmare haunted by purism, I do believe that having a function produce assertable outputs greatly aids in unit testing.

Assertable outputs may be in the form of data returned by the function, or if that is not available to us, observability via mocks of dependencies that are invoked within a method. Depending firstly on data returned by a function makes unit tests more robust to change, because we are not asserting what happens inside the function, rather, what is produced as a consequence of logical decisions made on the inside.

Did you notice that there was another error in our existing code? This was caused by a lack of functional clarity.

public async Task<FoodForJourney> GetFoodToFeedSlothsOnTheirJourney(int id, int numberOfSloths)
{
	try
	{
		return await client.FindFood($"select * from foodTable where rocketId = {id} and numberOfSlothsToCaterFor = {numberOfSloths}");
	}
	catch (Exception ex)
	{
		logger.LogError($"Exception caught while determining food to feed sloths. {ex.Message}.");

		// Red flag
		return new FoodForJourney();
	}
}

In the case where we fail to find food, we return a new FoodForJourney. But what does this actually mean?

namespace Simplicity.Database
{
    public record FoodForJourney()
    {
        public int NumberOfCourses { get; init; }
        public string[] Foods { get; init; }
    };
}

This class does not have any fields that indicate success (or failure for that matter). Furthermore, none of the fields are default initialised, meaning that the primitive type NumberOfCourses will be default initialised to 0, and the string[] will be initialised to null. Neither of these scream to us “Hey! We failed to find food!”.

The consuming code was actually written to expect a null value to be returned when no food is available, which obviously does not align with any output from the function.

In this case we have a couple of different options:

• Extend the FoodForJourney class to contain a field that indicates whether the retrieval of food was successful
• Return null to indicate that no food was found, because null is an expression that something does not exist. In this case the caller would ask for some food, and instead of actually getting food, they would get an indicator that food was not found – null.

Both of these solutions are adequate in order to give the consumer (and unit tests!) the clarity to understand that we have not obtained any food.

The Final View of Tests

At time of writing, we have 47 functional unit tests for our code.

I have explicitly highlighted core infrastructure types, such as the queue poller which require no tests, as they are pretending to be much more complex third party libraries.

Check out the repo of code from the code samples page to see what tests were written. There’s far too many to include in this post now!

Sloth Summary

What a journey! We have now:

• Learned the lateral lift and shift to solve constructor bloat
• Learned that there isn’t one single way to create a cohesive interface, which makes the practice of writing software both based in engineering and art/design
• Established that code should be as functional as possible, as having assertable outputs from methods makes unit testing them much easier
• Completed our refactoring and testing journey!