When optionals are… required

Assume you have developed a service, with methods requiring arguments, properly extracted into a interface. Then you (or your customers) would write nice clients for your service. Of course, such clients will probably be unit tested in isolation, such as using a generic framework like Moq. (Even if you have forgot to unit test yours!)

This small example – not very useful itself – illustrates the above:

public interface INumberWriter
{
  void WriteNumbers(int a, int b);
}

public class NumberWriter : INumberWriter
{
  public void WriteNumbers(int a, int b)
  {
    Console.WriteLine($"{a}, {b}");
  }
}

public class SubsequentNumberWriter
{
  private readonly INumberWriter writer;

  public SubsequentNumberWriter(INumberWriter writer)
  {
    this.writer = writer;
  }

  public void WriteSubsequentNumbers(int x)
  {
    writer.WriteNumbers(x, x + 1);
  }
}

class Program
{
  static void Main(string[] args)
  {
    var subsequentNumberWriter = 
      new SubsequentNumberWriter(new NumberWriter());
    subsequentNumberWriter.WriteSubsequentNumbers(1);
  }
}

As you can see, the client class was furthermore aggregated to an instance of your service by a main program (or better, dependency injected automatically).

What great software! The output is “1, 2“, as you would expect. And here is a simple verifying unit test for the beautiful SubsequentNumberWriter class too:

[TestMethod]
public void WriteSubsequentNumbers_For1_CallsServiceWith1And2()
{
  // Arrange
  var numberWriterMock = new Mock<INumberWriter>();
  numberWriterMock.Setup(m => 
    m.WriteNumbers(It.IsAny<int>(), It.IsAny<int>()));

  var subsequentNumberWriter = 
    new SubsequentNumberWriter(numberWriterMock.Object);

  // Act
  subsequentNumberWriter.WriteSubsequentNumbers(1);

  // Assert
  numberWriterMock.Verify(m => m.WriteNumbers(1, 2));
}

Now what? You’re right! It’s time for the next service improvement: it would be so nice if its clients could optionally indicate one more argument when they call WriteNumbers, as three numbers look so much better than only two, ain’t it? (Bear with me, don’t let you mind go to arrays for the purpose of the discussion.)

You adapt NumberWriter class and INumberWriter interface accordingly, considering that this way any existing clients, such as SubsequentNumbersWriter, won’t need any changes in order to continue to be built and run successfully:

public interface INumberWriter
{
  void WriteNumbers(int a, int b, int? c = null);
}

public class NumberWriter : INumberWriter
{
  public void WriteNumbers(int a, int b, int? c = null)
  {
    Console.Write($"{a}, {b}");
    if (c != null)
      Console.Write($", {c}");
    Console.WriteLine();
  }
}

Indeed, when you compile and run the client class and the main program that uses it, everything is fine. But – since I wrote this article – you surely suspect (and you are correct) that the change wasn’t as harmless as it looked like at first.

A problem appears if you’d try to compile… the unit test project. Specifically, you’d get:
Error CS0854 – An expression tree may not contain a call or invocation that uses optional arguments.

What’s the problem? You know it now: Moq framework is nice, but because it uses expressions to define setups and verifications (e.g. m =>
m.WriteNumbers(It.IsAny<int>(), It.IsAny<int>())), it doesn’t support optional arguments.

Instead, to make it work, you’ll be forced to update the unit test and indicate the third argument there, even if you do not care about it in the client class that is tested:

numberWriterMock.Setup(m => 
  m.WriteNumbers(It.IsAny<int>(), It.IsAny<int>(),
                 It.IsAny<int?>()));
// ...
numberWriterMock.Verify(m => 
  m.WriteNumbers(1, 2,
                 It.IsAny<int?>()));

Side note: here you may run into two other issues or questions (a little off topic, but I want to mention them because they are common):

  1. If you enter by mistake it is any int instead of it is any nullable int on the Setup call, the unit test will compile but the verify call will never succeed. And it’s going to be very frustrating as it will be very hard to detect why!
    • Mind that question mark, it’s very, very important!
  2. Should you use it is any in the verification call as well? Shouldn’t you verify passing of null? I believe not, myself, because you don’t test the core service here, but the client class, isn’t it?
    • Indeed, if the service implementation had a different default value for the optional argument, such as int? c = -1, while the interface would still have it set as int? c = null, the mock of the service would receive null, and the verify (with -1 in that case) would certainly fail. You can try it and see for yourself!

Now back to our main case: what’s the conclusion, one might ask.

Well, in my opinion, it’s just that it’s not a good decision to use optional arguments when you extend your services in subsequent version, i.e. adding arguments with default values to existing methods. At least not without considering this as a breaking change, like you would when you’d remove an argument from such a method instead.

(Note that although client unit testing with mocking is a great example of this issue appearing in practice, actually any expression using the method call would break similarly, and probably it will happen in way more important client code.)

What can you do instead? Surprise: plain old method overloading and a bit of code refactoring would turn out to be enough! Moreover, you can still use optional arguments – but only in private methods and maybe in the first version of an API; in public method updates, optionals should never be an option! 🙂

For our example, I’d suggest doing something like this:

public interface INumberWriter
{
  void WriteNumbers(int a, int b);
  void WriteNumbers(int a, int b, int c);
}

public class NumberWriter : INumberWriter
{
  public void WriteNumbers(int a, int b)
  {
    WriteNumbersCore(a, b);
  }
  public void WriteNumbers(int a, int b, int c)
  {
    WriteNumbersCore(a, b, c);
  }
  private static void WriteNumbersCore(int a, int b, int? c = null)
  {
    Console.Write($"{a}, {b}");
    if (c != null)
      Console.Write($", {c}");
    Console.WriteLine();
   }
 }

With this code updates, both the original client class and its unit test will compile successfully, and also will continue to pass. And your change is not going to be breaking anymore: the optionals shouldn’t ever be… required!

(Please excuse my ifs within WriteNumbers method bodies, and any other ugly C# that I didn’t care about. I’m sure you can refactor everything yourself.)

About Sorin Dolha

My passion is software development, but I also like physics.
This entry was posted in C# and tagged , , , . Bookmark the permalink.

Add a reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s