Constructor logic should be kept to a minimal, everyone knows that right? But what are the options of keeping the logic to a minimal but also allowing for the logic to not be duplicated through out your class. Let’s take a look!

Scenario #1

Every developer at some point in their career will have written a class of where the constructor takes a parameter, right?

public class Item
{
    public Item(string foo)
    {
        Foo = foo;
    }

    public string Foo { get; private set; }
}

So then you need to add validation to check whether the parameter being passed in is valid for your business logic needs and makes the instance valid.

So you add in a check.

Scenario #2

public class Item2
{
    public Item2(string foo)
    {
        if (string.IsNullOrWhiteSpace(foo))
        {
            throw new ArgumentException(nameof(foo));
        }

        Foo = foo;
    }

    public string Foo { get; private set; }
}

Now the parameter is checked and all is good.

The business changes the requirements and you need to be able to set the Foo property at other points of the system and the instance of your class still needs to be valid, so what do you do?

You’re pushed for time and you add a backing field to the auto property and perform the check right into the setter.

Scenario #3

public class Item3
{
    private string _foo;

    public Item3(string foo)
    {
        if (string.IsNullOrWhiteSpace(foo))
        {
            throw new ArgumentException(nameof(foo));
        }

        _foo = foo;
    }

    public string Foo
    {
        get => _foo;
        set
        {
            if (string.IsNullOrWhiteSpace(value))
            {
                throw new ArgumentException(nameof(value));
            }
            _foo = value;
        }
    }
}

But then the next sprint you’re given a new requirement that the foo of a valid object can’t allow spaces in, what do you do?

Scenario #4

public class Item4
{
    private string _foo;

    public Item4(string foo)
    {
        if (string.IsNullOrWhiteSpace(foo)
            && foo.IndexOf(" ", StringComparison.InvariantCultureIgnoreCase) > 0)
        {
            throw new ArgumentException(nameof(foo));
        }

        _foo = foo;
    }

    public string Foo
    {
        get => _foo;
        set
        {
            if (string.IsNullOrWhiteSpace(value)
                && value.IndexOf(" ", StringComparison.InvariantCultureIgnoreCase) > 0)
            {
                throw new ArgumentException(nameof(value));
            }
            _foo = value;
        }
    }
}

You update your logic in both places however now the same logic is in multiple places. This allows for code divergence to occur as well as violating the Single Responsibility Principle and adding technical debt to your codebase.

So what do you do? You abstract it into a private method.

Scenario #5

public class Item5
{
    private string _foo;

    public Item5(string foo)
    {
        if (ValidateFoo(foo))
        {
            throw new ArgumentException(nameof(foo));
        }

        _foo = foo;
    }

    public string Foo
    {
        get => _foo;
        set
        {
            if (ValidateFoo(value))
            {
                throw new ArgumentException(nameof(value));
            }
            _foo = value;
        }
    }

    private bool ValidateFoo(string foo)
    {
        return (string.IsNullOrWhiteSpace(foo)
                && foo.IndexOf(" ", StringComparison.InvariantCultureIgnoreCase) > 0);
    }
}

You’ve got the logic in a single place and you’re feeling good about yourself. However the logic to throw the exception is duplicated and it is being called multiple times. So we move this into the validate method.

Scenario #6

public class Item6
{
    private string _foo;

    public Item6(string foo)
    {
        ValidateFoo(foo);

        _foo = foo;
    }

    public string Foo
    {
        get => _foo;
        set
        {
            ValidateFoo(value);
            _foo = value;
        }
    }

    private void ValidateFoo(string foo)
    {
        if (string.IsNullOrWhiteSpace(foo)
            && foo.IndexOf(" ", StringComparison.InvariantCultureIgnoreCase) > 0)
        {
            throw new ArgumentException(nameof(foo));
        }
    }
}

But you look at the code and think “but I’ve now got duplicate calls to the logic” and what happens if another method needs to set _foo the developer needs to remember to call the validate method. This will probably be missed and you could get into an inconsistent state; not good! Also you’ve got a private method which, in a large class, cause unnessessary noise.

So what do you do? Move the logic into the setter!

Scenario #7

public class Item7
{
    private string _foo;

    public Item7(string foo)
    {
        Foo = foo;
    }

    public string Foo
    {
        get => _foo;
        set
        {
            if (string.IsNullOrWhiteSpace(value)
                && value.IndexOf(" ", StringComparison.InvariantCultureIgnoreCase) > 0)
            {
                throw new ArgumentException(nameof(Foo));
            }
            _foo = value;
        }
    }
}

You update the constructor to set the private member variable via the property of Foo and the conditional logic gets executed. It is in a single location, abstracted away from the inner (and outer) workings of the class and is easy to find.

Setting the foo is now done through the property, whether in the constructor or setting the Property directly.

Downside

There is a downside to this structure. You may have updated the constructor to use the single code path. You have also updated the code which consumes instances of this class to use the single code path. However you have not taken into account that developers may add functionality and access the field directly in the future. This code will not stop this, however that is what code reviews are for.

You can make it happen however it’s a bit forced and I would not recommend it. It is over engineered and if you don’t trust your developers to do the right thing then there are probably bigger issues to resolve.

But for completeness this is how you can do it.

public abstract class Item7Base
{
    private string _foo;

    protected Item7Base(string foo)
    {
        Foo = foo;
    }

    public string Foo
    {
        get => _foo;
        set
        {
            if (string.IsNullOrWhiteSpace(value)
                && value.IndexOf(" ", StringComparison.InvariantCultureIgnoreCase) > 0)
            {
                throw new ArgumentException(nameof(Foo));
            }
            _foo = value;
        }
    }
}

public class Item7 : Item7Base
{
    public Item7(string foo) : base(foo)
    {
    }
}

Conclusion

I think sometimes in modern C# development backing fields are forgotten about or frowned upon with the new language features eg. auto properties, expression bodies etc. available. Remember backing fields aren’t bad. The compiler puts them in anyway for auto properties as the auto properties are just syntatic sugar. However personally there needs to be a time and place to add them in manually.

This is just one example of applying principles to keep clean and maintainable code. This solution will not be applicable for all and it will depend on what you are developing as well as what your coding standards are. It’s just another option!

Let me know what you think! Any questions/comments then please contact me on Twitter @WestDiscGolf