Search code examples
c#.netstructdefault

How to avoid primitive obsession correctly with handling of `default(T)`?


I want to avoid primitive obsession with following struct. There are two goals why I'm doing this:

  • make method signatures more honest
  • ensure that invalid value can't exist

Implementation:

public struct SomeId : IEquatable<SomeId>
{
    public static readonly SomeId Empty = new SomeId(String.Empty);

    private SomeId(string someId)
    {
        Value = someId;
    }

    public string Value { get; }

    public static bool operator ==(SomeId left, SomeId right) => left.Equals(right);
    public static bool operator !=(SomeId left, SomeId right) => !(left == right);
    public static implicit operator string(SomeId id) => id.Value;
    public override int GetHashCode() => Value.GetHashCode();
    public override bool Equals(object obj) => Value.Equals(obj);
    public bool Equals(SomeId other) => String.Equals(Value, other.Value, StringComparison.Ordinal);

    public static SomeId? Create(string value)
    {
        if (String.IsNullOrWhiteSpace(value))
        {
            return new SomeId(value);
        }

        return null;
    }
}

Quite a lot of code but still not perfect! Indeed it solves main issue that I won't pass strings all over the place but give methods meaningful signature.

Still, I'm able to sneak invalid value into my application just by creating new instance by default(SomeId) - since Value is of type string, I will get SomeId in invalid state with Value = null.

What would be the best solution here? Should I even care?

Sure I could do for example this:

private string _value;
public string Value => _value ?? String.Empty

... but that additional null check whenever I access this property is bugging me.


Solution

  • Should you care? Yes, I think so. It's very easy to create zero-initialised structs (default(SomeId), new SomeId(), new SomeId[n]), and your struct is semantically invalid in that state.

    You have a few options:

    • Null coalescing in the getter (your proposed solution). You are right, if the field is null, that will always result in a few more instructions to execute. The question is, do those extra instructions (e.g. load null, compare for equality, load static field) have a measurable impact on execution speed?
    • Check for null in the getter and set the field to string.Empty if required. Technically it's a getter with side effects (even though the data are encapsulated), which some people have strong opinions on, but you can also call it lazy initialisation.
    • Declare default instances to be invalid, like ImmutableArray<T> does.