Test code needn’t be defensive

In a code review I encountered some test code that looked a bit like this:

 var result = await _controller.Resources() as ViewResult;
 result.Should().NotBeNull();
 // ReSharper disable once PossibleNullReferenceException
 result.Model.Should.BlahBlahBlah();
 

This is a typical defensive coding pattern whose reasoning goes like this:

  • The return type of _controller.Resources() is Task<ActionResult>.
  • I need to cast the inner Result of this Task to a ViewResult, as I want to inspect its Model attribute.
  • But the Result could be a different subclass of ActionResult, so I had better use a safe cast, just in case.
  • As I’m using a safe cast, I can’t guarantee that I’ll get any instance back, so I had better do a null check.
  • Oh look! ReSharper is complaining when I try to access properties of this object. As I’ve already performed a null check, I’ll turn off the warnings with a comment.

Now, defensive coding styles are valuable when we don’t know what data we’ll be handling, but this is most likely to happen at the boundaries of a system, where it interacts with other systems or, even more importantly, humans.

But in the context of a unit test, things are different:

  • We are in control of both sides of the contract: the test and class under test have an intimate and interdependent existence. A different type of response would be unexpected and invalid.
  • An attempt to directly cast to an invalid type will throw a runtime error, and a runtime error is a meaningful event within a test. If _controller.Resources() returns any other subclass of ActionResult, then the fact that it cannot be cast to ViewResult is the very information I want to receive, as it tells me how my code is defective.

This means I can rewrite the code like this:

 
var result = (ViewResult) await _controller.Resources(); 
result.Model.Should.BlahBlahBlah();

By setting aside the defensive idiom, I’ve made the test clearer and more precise, without losing any of its value.

Identical random values in parallel NUnit test assemblies

We have some NUnit cross-system test assemblies that run in parallel. They upload files and watch to see how they are processed. We were inserting a randomly generated value into the filenames in an attempt to avoid identically named files overwriting each other.

Unfortunately, this didn’t work, and we saw frequent test failures. In particular, we found that filenames generated by one test assembly were often identical to those generated by another.

I wanted to understand why this was happening, so I did some investigation.

We were getting our values fromTestContext.CurrentContext.Random, which is an instance of NUnit’s Randomizer class.

When we look at the implementation of Randomizer, we see this:

static Randomizer()
{
    InitialSeed = new Random().Next();
    Randomizers = new Dictionary<MemberInfo, Randomizer>();
}

The Randomizer is statically seeded with a value generated by the System.Random class. Because this seed is static, the same sequence of random values is shared by all references to Randomizer within each assembly, producing a sequence of values that are highly likely to be different from each other. However, references to Randomizer in a concurrently running assembly use a different instance and have their own sequence of values with its own seed.

Let’s have a quick look at how System.Random is seeded.

The MSDN documentation tells us:

  • The Random() constructor uses the system clock to provide a seed value. This is the most common way of instantiating the random number generator.

And goes on to warn:

…because the clock has finite resolution, using the parameterless constructor to create different Random objects in close succession creates random number generators that produce identical sequences of random numbers.

It would seem that our two parallel test assemblies often start executing within such a short interval of each other, and that NUnit’s Randomizer is seeded with the same value in each assembly, which means the sequences of values are identical.

There is some discussion of introducing a mechanism for controlling the seeding of Randomizer in NUnit, but in the mean time, the solution to our problem was to seed our own System.Random instances, rather than relying on NUnit’s.

How applying Theory of Constraints helped us optimise our code

The neck of a bottle of prosecco in front of a fire.

My team have been working on improving the performance our API, and identified a database call as the cause of some problems.

The team suggested three ways to tackle this problem:

  • Scale up the database till it can meet our requirements.
  • Introduce some light-weight caching in the application to reduce load on the database.
  • Examine the query plan for this database call to find out whether the query can be optimised.

Which of these should we attempt first? There was some intense discussion about this, with arguments made in favour of each approach. What we needed was a simple framework for making decisions about how to improve our system.

This is where the Theory of Constraints (ToC) can help. Originally expounded as a paradigm for improving manufacturing systems, ToC is really useful in software engineering, both when managing projects and when improving the performance of the systems we create.

Theory of Constraints

The preliminary step in applying ToC is to identify the Goal of your system. In the case of this API, the Goal is to supply accurate data to consumers.

Now we understand the Goal of the system, we can define the Throughput of the system as the rate at which it can deliver units of that goal, in our case API responses. We can also define the Operating Expenses of the system (the cost of servers) and its Inventory (requests waiting for responses).

The next step is to identify the Constraint of the system. This is the element in the system that dictates the system’s Throughput. In a physical system, a useful heuristic is a build-up of Inventory in front of this element. In our API, our monitoring helped us pinpoint the bottleneck.

The next three steps give us a sequence of approaches for tackling the Constraint:

  • First, Exploit the Constraint by finding local changes you can make to improve its performance.
  • Second, Subordinate the rest of the system to the Constraint by finding ways to reduce pressure on it so it can perform more smoothly.
  • Third, Elevate the Constraint by increasing the resources available to it, committing to additional Operating Expenses if necessary.

Exploitation comes first because it’s quick, cheap and local. To Subordinate you need to consider the effects on the rest of the system, but there shouldn’t be significant costs involved. Elevating the Constraint may well cost a fair amount, so it comes last on the list.

Once you have applied these steps you will either find that the Constraint has moved elsewhere (you’ve ‘broken’ the original Constraint), or it has remained in place. In either case, you should repeat the steps as part of a culture of continuous improvement. Eventually you want to see the constraint move outside your system and become a matter of consumer demand.

Applying ToC to our question

If we look at the team’s three suggestions, we can see that each corresponds to one of these techniques:

  • Scaling up the database is Elevation: there’s a clear financial cost in using larger servers.
  • Introducing caching is Subordination: we’re changing the rest of the system to reduce pressure on the Constraint, and need to consider questions such as cache invalidation before we make this change.
  • Optimising the query is Exploitation: we’re making local changes to the Constraint to improve its performance.

Applying ToC tells us which of these approaches to consider first, namely optimising the query. We can look at caching if an optimised query is still not sufficient, and scaling should be a last resort.

In our case, query optimisation was sufficient. We managed to meet our performance target without introducing additional complexity to the system or incurring further cost.

Further Reading

Goldratt, Eliyahu M.; Jeff Cox. The Goal: A Process of Ongoing Improvement. Great Barrington, MA.: North River Press.

Concrete Inheritance and the Dependency Inversion Principle

Collaboration and DIP

We know that this code violates the Dependency Inversion Principle (DIP):

public class Atm
{
    private readonly InMemoryTransactions _inMemoryTransactions;

    public Atm()
    {
        _inMemoryTransactions = new InMemoryTransactions(new List<Transaction>());
    }

    public void Deposit(int amount)
    {
        _inMemoryTransactions.Deposit(amount);
    }
}

In particular, line 3 violates the principle by creating a dependency on a specific implementation, and line 7 violates it by knowing how to construct that dependency.

We can rewrite this code to apply the DIP:

public class Atm
{
    private Transactions _transactions;

    public Atm(Transactions transactions)
    {
        _transactions = transactions;
    }

    public void Deposit(int amount)
    {
        _transactions.Deposit(amount);
    }
}

Line 3 now depends on an abstraction, and we use constructor injection to supply this dependency.

Inheritance and DIP

So how about this code:

public class LoggingInMemoryTransactions : InMemoryTransactions
{
    private readonly Logger _logger;

    public LoggingInMemoryTransactions(List<Transaction> initialTransactions, Logger logger) : base(initialTransactions)
    {
        _logger = logger;
    }

    public override void Deposit(int amount)
    {
        _logger.Log($"Deposit {amount}");
        base.Deposit(amount);
    }
}

Well, this class depends on a Logger abstraction, which is injected in the constructor, so that dependency seems to have been satisfactorily inverted.

However, in line 1 we can see that this class inherits from InMemoryTransactions. This inheritance is also a dependency, and we’re inheriting from a concrete instance, not an abstraction.

If we then look at the constructor on line 5, we can see that it calls the base constructor; in other words, it has to know how its base class is instantiated.

There is a direct parallel between these two cases: each of them has a dependency on a specific concrete class, which is instantiated in a specific way.

We can rewrite this code to use composition rather than inheritance:

public class LoggingInMemoryTransactions : Transactions
{
    private readonly Logger _logger;
    private readonly Transactions _transactions;

    public LoggingInMemoryTransactions(Logger logger, Transactions transactions)
    {
        _logger = logger;
        _transactions = transactions;
    }

    public void Deposit(int amount)
    {
        _logger.Log($"Deposit {amount}");
        _transactions.Deposit(amount);
    }
}

This simple example of the Decorator pattern behaves in exactly the same way as the previous implementation, but is now only dependent on abstractions.

Unit Testing

If we try to write unit tests around our code, we will soon see the benefit of DIP.

If we try to test our first implementation of Atm, we find that our test boundary includes InMemoryTransactions, as that class is a hard-wired dependency. We can’t test the behaviour of   Atm without also testing the behaviour of  InMemoryTransactions. If we also have tests of InMemoryTransactions, then we may end up duplicated test scenarios.

In this case we have an additional problem: we have only defined read access to the data, so we have no way of testing this class without violating encapsulation:

using NUnit.Framework;

[TestFixture]
public class AtmShould
{
    [Test]
    public void Perform_deposit_transaction()
    {
        const int amount = 100;
        var atm = new Atm();

        atm.Deposit(amount);

        // HELP! WHAT CAN I ASSERT AGAINST?
    }
}

In the second implementation, we can write collaboration tests between Atm and Transactions, bringing our test boundary in much closer, and restricting the behaviour under test to that of  Atm.

using NSubstitute;
using NUnit.Framework;

[TestFixture]
public class AtmShould
{
    [Test]
    public void Perform_deposit_transaction()
    {
        const int amount = 100;
        var transactions = Substitute.For<Transactions>();
        var atm = new Atm(transactions);

        atm.Deposit(amount);

        transactions.Received().Deposit(amount);
    }
}

Similarly, to test the first implementation of LoggingInMemoryTransactions, we also have to test the behaviour it inherits from InMemoryTransactions, as one is inseparable from the other. Again, if we also have tests of the base class, then we may end up with duplicated test scenarios.

using NSubstitute;
using NUnit.Framework;

[TestFixture]
public class LoggingInMemoryTransactionsShould
{
    private LoggingInMemoryTransactions _loggingInMemoryTransactions;
    private Logger _logger;

    private const int Amount = 100;

    [SetUp]
    public void SetUp()
    {
        _logger = Substitute.For<Logger>();
        _loggingInMemoryTransactions = new LoggingInMemoryTransactions(new List<Transaction>(), _logger);
    }

    [Test]
    public void Log_transaction()
    {
        _loggingInMemoryTransactions.Deposit(Amount);

        _logger.Received().Log($"Deposit {Amount}");
    }

    [Test]
    public void Store_deposit()
    {
        _loggingInMemoryTransactions.Deposit(Amount);

        // HELP! HOW CAN I CHECK THIS WAS SAVED?
        // I might be tempted to assert against the new List<Transaction>(), but this would leak implementation details.
    }
}

In the second implementation, the specific behaviour of LoggingInMemoryTransactions is separated, and we can write collaboration tests independently of the behaviour of the inner implementation.

using NSubstitute;
using NUnit.Framework;

[TestFixture]
public class LoggingInMemoryTransactionsShould
{
    private LoggingInMemoryTransactions _loggingInMemoryTransactions;
    private Logger _logger;
    private Transactions _transactions;

    private const int Amount = 100;

    [SetUp]
    public void SetUp()
    {
        _logger = Substitute.For<Logger>();
        _transactions = Substitute.For<Transactions>();
        _loggingInMemoryTransactions = new LoggingInMemoryTransactions(_logger, _transactions);
    }

    [Test]
    public void Log_transaction()
    {
        _loggingInMemoryTransactions.Deposit(Amount);

        _logger.Received().Log($"Deposit {Amount}");
    }

    [Test]
    public void Store_deposit()
    {
        _loggingInMemoryTransactions.Deposit(Amount);

        _transactions.Received().Deposit(Amount);
    }
}

 

Updated 26 May 2016: I’ve given code examples for the unit tests we might write in each case, and have removed DateTime createdOn from the example to avoid confusing the issue.

Updated 26 September 2017: I’ve removed a reference to an individual whose political views are not consistent with the spirit of this blog.

A strange behaviour in ReSharper’s Move Instance Method refactoring

My colleague Pedro and I were puzzling over some bizarre behaviour with ReSharper’s Move Instance Method refactoring move. This is a fairly complex move, and it makes various changes to the source and destination classes; however, in many cases, it generates code that does not compile, and seems to include wrongly qualified references.

Here is an example drawn from Martin Fowler’s Refactoring: Improving the Design of Existing Code:

Employee class before refactoring

using System;

namespace RefactoringExamples.ReplaceConditionalWithPolymorphism
{
    public class Employee
    {
        private EmployeeType _employeeType;
        private readonly int _monthlySalary;
        private readonly int _commission;
        private readonly int _bonus;

        public Employee(int type)
        {
            Type = type;
            _monthlySalary = 100;
            _commission = 10;
            _bonus = 20;
        }

        public int Type
        {
            get { return _employeeType.TypeCode; }
            set { _employeeType = EmployeeType.TypeFrom(value); }
        }

        public int PayAmount()
        {
            return Pay();
        }

        private int Pay()
        {
            switch (_employeeType.TypeCode)
            {
                case EmployeeType.Engineer:
                    return _monthlySalary;
                case EmployeeType.Salesperson:
                    return _monthlySalary + _commission;
                case EmployeeType.Manager:
                    return _monthlySalary + _bonus;
                default:
                    throw new Exception("Incorrect Employee");
            }
        }
    }
}

 EmployeeType class before refactoring:

using System;

namespace RefactoringExamples.ReplaceConditionalWithPolymorphism
{
    public abstract class EmployeeType
    {
        public abstract int TypeCode { get; }

        public static EmployeeType TypeFrom(int value)
        {
            switch (value)
            {
                case Engineer:
                    return new Engineer();
                case Salesperson:
                    return new Salesperson();
                case Manager:
                    return new Manager();
                default:
                    throw new Exception("Incorrect Employee Code");
            }
        }

        public const int Engineer = 0;
        public const int Salesperson = 1;
        public const int Manager = 2;
    }

    class Engineer : EmployeeType
    {
        public override int TypeCode => Engineer;
    }

    class Salesperson : EmployeeType
    {
        public override int TypeCode => Salesperson;
    }

    class Manager : EmployeeType
    {
        public override int TypeCode => Manager;
    }
}

Employee class after refactoring

using System;

namespace RefactoringExamples.ReplaceConditionalWithPolymorphism
{
    public class Employee
    {
        private EmployeeType _employeeType;
        private readonly int _monthlySalary;
        private readonly int _commission;
        private readonly int _bonus;

        public Employee(int type)
        {
            Type = type;
            _monthlySalary = 100;
            _commission = 10;
            _bonus = 20;
        }

        public int Type
        {
            get { return _employeeType.TypeCode; }
            set { _employeeType = EmployeeType.TypeFrom(value); }
        }

        public int MonthlySalary
        {
            get { return _monthlySalary; }
        }

        public int Commission
        {
            get { return _commission; }
        }

        public int Bonus
        {
            get { return _bonus; }
        }

        public int PayAmount()
        {
            return _employeeType.Pay(this);
        }
    }
}

EmployeeType class after refactoring

using System;

namespace RefactoringExamples.ReplaceConditionalWithPolymorphism
{
    public abstract class EmployeeType
    {
        public abstract int TypeCode { get; }

        public static EmployeeType TypeFrom(int value)
        {
            switch (value)
            {
                case Engineer:
                    return new Engineer();
                case Salesperson:
                    return new Salesperson();
                case Manager:
                    return new Manager();
                default:
                    throw new Exception("Incorrect Employee Code");
            }
        }

        public const int Engineer = 0;
        public const int Salesperson = 1;
        public const int Manager = 2;

        public int Pay(Employee employee)
        {
            switch (System.TypeCode)
            {
                case EmployeeType.Engineer:
                    return employee.MonthlySalary;
                case EmployeeType.Salesperson:
                    return employee.MonthlySalary + employee.Commission;
                case EmployeeType.Manager:
                    return employee.MonthlySalary + employee.Bonus;
                default:
                    throw new Exception("Incorrect Employee");
            }
        }
    }

    class Engineer : EmployeeType
    {
        public override int TypeCode => Engineer;
    }

    class Salesperson : EmployeeType
    {
        public override int TypeCode => Salesperson;
    }

    class Manager : EmployeeType
    {
        public override int TypeCode => Manager;
    }
}

Notice the reference to System.TypeCode in the EmployeeType.Pay method on line 30 above; this doesn’t compile, and is most definitely not what we wanted. My hypothesis is that once ReSharper has rewritten the code, it attempts to resolve and properly qualify any symbol names; in this case, it spots TypeCode and, because this is the name of a type known to the compiler, it decides that the name needs to be appropriately qualified, adding the System namespace. It’s failing to recognise that TypeCode is actually the name of a property on the EmployeeType class, and that no qualification is necessary.

I have found three ways to work round this:

First, you can give the property a name that not already used by a type known to the compiler. If I rename TypeCode to Code, then the refactoring move works perfectly. In this case, this is probably a nice idea, as the name EmployeeType.TypeCode contains redundancy. However, in many cases the most appropriate name for a property may coincide with a type name, and renaming it will not be a good option.

The second option is to recognise that this happens, and fix it. In this case, all I need to do is remove the System namespace on line 30, and everything works correctly. I’m not a huge fan of this solution, as I like my automatic refactorings to be safe and reliable, but it may be a pragmatic choice.

The third option is to perform this refactoring by composing smaller, safe steps. This can give you finer grained control over the outcome, but at the expense of complexity.

I would be interested to hear if anyone has any further insight into what’s going on here, or if there are other techniques for overcoming it. Do let me know if you do!

Update (25 May 2016): I sent JetBrains a link to this post, and within an hour they raised a bug report. Thanks to Daria at JetBrains for the speedy response!

Reinvigorating a daily stand-up by walking the board

I’ve been working with a team who had a problem with focus: when I joined them, they seemed to be busy all the time, but they were frustrated that they weren’t making progress towards their sprint goals.

This situation could be seen in microcosm at the daily stand-up meeting. In this post I’m going to describe how a simple adjustment to this meeting helped us start to improve focus, morale and productivity.

The Scrum Guide gives a template for the daily stand-up meeting (which it calls the Daily Scrum):

The Daily Scrum is held at the same time and place each day to reduce complexity. During the meeting, the Development Team members explain:

  • What did I do yesterday that helped the Development Team meet the Sprint Goal?
  • What will I do today to help the Development Team meet the Sprint Goal?
  • Do I see any impediment that prevents me or the Development Team from meeting the Sprint Goal?

My team was indeed practising this technique, but it seemed that they often forgot the discipline of focusing on ‘helping the Development Team meet the Sprint Goal’, and the meeting often descended into yesterday-I-diddery, with each team member recounting all the things they did the day before in trivial detail.

It seems to me that in a stand-up of this format, each team member’s incentive becomes having something to say, rather than showing progress towards the Sprint Goal, and this produces an incentive to be busy, no matter how irrelevant or frankly counterproductive the tasks might be. In this team, I saw a lot of effort spent on support tasks—whether or not the issue was pressing—, a significant amount of aimless ‘refactoring’, which was essentially yak shaving, and a tendency for team members to interrupt each other for help with lower priority work. In effect, everyone starts prioritising busy work, rather than focusing on the team’s goals.

The other consequence of this approach was that the team’s board was a poor representation of our work: people would be working on tasks that weren’t visible on the board, and the stories that were on the board often didn’t move anywhere. The Scrum Master and I tried various approaches to coordinate the board and the stand-up reports, but the focus was still lacking.

I’ve previously worked in a Kanban environment, and the format of a Kanban standup is significantly different:

Standup meetings have evolved differently with Kanban. The need to go around the room and ask the three questions is obviated by the card wall. … The focus is on flow of work. The facilitator … will “walk the board.” The convention has developed work backward—from right to left (in the direction of pull)—through the tickets on the board. The facilitator might solicit a status update on a ticket or simply ask if there is any additional information that is not on the board and may not be known to the team.

(Anderson, David J. Kanban, Successful evolutionary change for your technology business)

I suggested that we try this approach for a week, and see whether it helped give us more focus. As people were concerned that we might lose sight of important work, we agreed that we would walk the board first, and then run quickly round the team to see if anything was missing.

The initial results were encouraging, and several weeks later we are still walking the board, rather than going round the team. In particular, our board now contains a great deal more information on the tasks in play, and the team have got really good at carding up even small tasks so they are visible at the next stand-up. The amount of off-plan and busy work has also dropped, and this also be a result of the focus on the tasks on the board. Perhaps my favourite development is that the tasks on the board have become much smaller: the drive to get things done is now focused on pulling small, focused tasks across the board, rather than doing busy work.

Of course this technique is no cure-all, and it took a while for the team to acquire the discipline of walking the board in order, rather than jumping in to discuss whichever task particularly excites them. However, as an incremental adaptation, I’m very pleased with its results.

A Retrospective in the Park

The other day, I facilitated a sprint retrospective in the park. The sun was shining, and we had all been working hard to complete our backlog, so it felt like a nice reward for everyone’s efforts. Holding a retrospective outdoors can also give it an energy and sense of enthusiasm that is harder to find in a small room.

I’ve run outdoor retrospectives before, and have previously followed fairly classic plans, with much arranging of index cards. This has never been a great success, as the slightest breath of a breeze can make a mess of your planning. For this retrospective, I designed a plan to avoid these problems, drawing some ideas from the Appreciative Retrospective plan.

This retrospective took an hour for a team of nine. You’ll need a pile of index cards or sticky notes, and a pen per person. Here’s how to do it:

1. Choose your location

Some people are sun lovers, whilst others, like me, burn easily and need some shade, so find a location that will work for everyone. Don’t worry too much about the state of the grass, as I suggest you conduct the retrospective standing up, if possible.

Get everyone to stand in a circle, with enough personal space for everyone, but close enough that you can hear everyone speak.

Observations: It’s nice if you can find a location where there won’t be too many distractions. We weren’t entirely successful: there was a hen party in another corner of the park, whose popping of prosecco corks and parading of an anatomically exaggerated blow-up mannequin was hard to ignore; there was also a group of male models sunning themselves noisily behind us (I’m working at a fashion company, so this is less unusual than it may sound), and three young women were smoking some interesting cigarettes upwind of us. Nonetheless, our retrospective was a success despite occasional distractions.

2. Characterise the sprint

Ask everyone to spend a couple of minutes coming up with three words to characterise the sprint; then go clockwise round the circle and ask each person to tell the team their three words.

Observations: It’s surprising how much difficulty people have sticking to three words; the important focus of this task is not the three-word limit, but getting a concise summary of the sprint.

3. Thank your neighbour

Moving anticlockwise around the team, ask each team member to thank their neighbour for something they have done during this sprint.

Observations: Our Scrum Master broke the rules by thanking the whole team for their efforts. The focus of this task is to generate a positive mood across the team, and it’s important that no one misses out on individual thanks, so I asked him to thank his neighbour for something as well.

4. Describe what went well

Hand each person three cards, and give them three minutes to write down three things that went well during the sprint. Going clockwise round the circle from a different starting point, ask each person to read out their three successes.

Observations: It’s useful for the facilitator to observe and comment on common themes, as this can help reinforce good practice.

5. Describe what could improve

Hand each person three more cards, and give them another three minutes to write down three things that could have gone even better during the sprint. Then go anticlockwise round the circle and ask each person to read out their three improvements.

6. Group the improvements

Instead of arranging the cards on a whiteboard (which isn’t practical in the park), appoint a champion for each improvement. Ask the first person to choose one of the improvements they suggested, and then get everyone else to hand this person any cards that describe a similar improvement. Keep running round the team until each team member has just one group of cards.

Observations: There’s a chance that you’ll end up with more themes than team members, in which case you’ll have to make a decision to drop some of these themes; in our case we had fewer common themes than team members, so we didn’t have to do this.

7. Select and discuss the most common themes

Rather than dot-voting, which again is impractical in the park, select the commonest themes. Ask everyone with any cards to take a step into the circle. Then ask everyone with just one card to take a step out again; then everyone with just two cards; then three, and so on until just three people are left in the inner circle.

Then have a three-minute discussion of each of these suggested improvements, with the focus of identifying at least one action per theme for the next sprint. Ensure someone is assigned to each action.

Observations: We ran slightly over the three minutes assigned to each theme, but this wasn’t a problem; if we hadn’t had a time limit, I suspect the conversation would have been much less focused.

8. Round off the retrospective

Finally, going round the circle clockwise, ask everyone to describe how they felt about the retrospective itself.

Observations: The feedback was very positive. The team had clearly enjoyed the opportunity to get out of the office, and they felt that the session had been successful: everyone was engaged and we came up with some good actions.

Feature Triggers

When we’re continuously integrating code changes, we may want to avoid prematurely exposing new behaviour to our users. A popular technique is to use Feature Toggles to turn a feature on based on a configuration setting. These are certainly useful, and are great for gradually rolling features out, but they involve introducing code into our systems that is not relevant to the system itself, and if poorly implemented can lead to us bloating our codebases with vapourware.

There is another technique that I think deserves more attention; I refer to this as Feature Triggers. The idea is to use an intrinsic part of your system to enable the new feature, rather than an extrinsic piece of configuration. The discipline lies in identifying the final change that will trigger the feature.

In his write-up of Feature Toggles, Martin Fowler describes a version of this technique that focuses on UI changes:

Release toggles are a useful technique and lots of teams use them. However they should be your last choice when you’re dealing with putting features into production.

Your first choice should be to break the feature down so you can safely introduce parts of the feature into the product. The advantages of doing this are the same ones as any strategy based on small, frequent releases. You reduce the risk of things going wrong and you get valuable feedback on how users actually use the feature that will improve the enhancements you make later.

If you really must hide a partly built feature, then the best way is to build all of it save the UI entry point and add that UI in a single release cycle. This way the non-ui code is fully integrated with everything else, but nothing is visible or used until you add the last bit at the end.

(Emphasis mine)

However, we needn’t restrict this technique to the UI. In particular, if you are practising Feature Pull, then the trigger should be the last piece of work in the chain, rather than necessarily a UI change.

Let me give an example.

I worked on a system that sold recorded music on line, and we wanted to start selling high-resolution tracks. The system had theretofore only supported a single audio format per track, usually 320 bps MP3, so the UI simply presented the customer with a ‘Add to basket’ button. Now we wanted to present them with a choice of, say, MP3 or FLAC, at different prices.

Our feature flow worked something like this:

  • When a customer has a track as a FLAC in their library, then they can download and listen to it with a FLAC codec.
  • When a customer has a track as a FLAC in their shopping basket, and they check out, it ends up in their library.
  • When a customer on the website clicks to add a track to their basket as a FLAC, then the track ends up in their shopping basket as a FLAC.
  • When a track is in the catalogue with a FLAC format, then the website shows that it can be added to basket as a FLAC.
  • When we have negotiated FLAC rights for a track that is ingested, then that track ends up in the catalogue with a FLAC format.

Now, each of these steps could be implemented in turn, using the principle of fake it till you make it to drive out the interface and schema changes as we went along. Crucially, we maintained existing behaviour for existing data. In the case of the UI, this meant that when a track was available in a single format, we just showed an ‘add to basket’ button with format-agnostic behaviour, whereas a track with multiple formats triggered a slightly more complex UI whose buttons had content-specific behaviour. Notice that in this case the UI was not the last piece of the puzzle: it’s behaviour was triggered by upstream data.

Instead, the entirety of this new feature was triggered by enabling the new ingestion rules—which then enabled multiple formats in the catalogue—which then enabled multiple formats on the website—which then enabled tracks to be added to the basket with specified formats—which then enabled format-aware tracks in the library—which then enabled format-aware downloads. All through this we limited the vapidity of the feature by keeping the code branching to a minimum and by releasing continuously, which meant that new code was almost immediately in use in production.

I’m sure this technique is widespread, and I think it’s worth reminding ourselves that the trigger needn’t be in the UI: by combining this technique with Feature Pull, we can often do without feature toggles and write more focused code.