A number of times over the years I have seen code which wants to be defensive (which is good) and check inputs to methods however isn’t using the power of the framework to help.

A number of basic building blocks, such as Guid and int, have such methods which can help - they are the TryParse methods.

The Problem

I’d imagine we’ve all seen code similar to this over the years.

public void MethodName(string input)
{
    if (string.IsNullOrWhiteSpace(input))
    {
        throw new ArgumentNullException();
    }

    Guid parsed;

    try
    {
        parsed = Guid.Parse(input);
    }
    catch (FormatException e)
    {
        throw new ArgumentNullException();
    }
    
    if (parsed == Guid.Empty)
    {
        throw new ArgumentNullException();
    }

    // do the actual work here
}

Let me start off by saying the above isn’t completely bad. Checking at various points works great. The one part which is “demoware” is where the FormatException is caught and an ArgumentNullException is thrown. This can be replaced by a more appropriate exception however a FormatException can imply an ArgumentNullException so it’s not all bad.

How can this be improved?

Unit test

First we need a unit test to prove that everything stays the same as we refactor. As you can see from the unit test this is why FormatException was caught and an ArgumentNullException was thrown.

[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData("1234589")]
[InlineData("qwertyuiop")]
[InlineData("00000000-0000-0000-0000-000000000000")]
public void Test(string input)
{
    Action result = () => MethodName(input);

    result.Should().Throw<ArgumentNullException>();
}

Using xUnit and the Theory/InlineData attribute combinations you can test multiple scenarios with only 1 test body.

Potential Solution

public void MethodName(string input)
{
    if (string.IsNullOrWhiteSpace(input))
    {
        throw new ArgumentNullException();
    }

    if (!Guid.TryParse(input, out var parsed))
    {
        throw new ArgumentNullException();
    }

    if (parsed == Guid.Empty)
    {
        throw new ArgumentNullException();
    }

    // do the actual work here
}

With Guid.TryParse and some modern inline out parameter syntax it looks a lot better; cleaner, easier to read and simpler to maintain.

The issue with above is you are checking the input variable multiple times. We’re checking before we use TryParse and then inside the inner workings of TryParse it is checking again.

So let’s clean it up a bit more.

public void MethodName(string input)
{
    if (!Guid.TryParse(input, out var parsed) || parsed == Guid.Empty)
    {
        throw new ArgumentNullException();
    }

    // do the actual work here
}

Conclusion

As you can see it’s easy to work with the framework tools to make code easier to read and maintain. This also allows us to concentrate on writing functionality and adding value instead of duplicating framework code.

The above is an example of how code can be built up and unit tested during development and lends itself well to being refactored before shipping.

Remember; Red, Green, Refactor!

Any questions/comments then please contact me on Twitter @WestDiscGolf