Tweeted twitter.com/StackSoftEng/status/1039302764080390144
2 fixed grammar
source | link
  1. Was creating an Airport class or struct a good solution in general, or am I solving the wrong problem / solving it the wrong way by creating the type? If it's not a good solution, what is a better solution?

  2. How should my application handle instances where the default(Airport) is used? A type of default(Airport) is nonsensical to my application, so I've been doing if (airport == default(Airport) { throw ... } in places where getting an instance of Airport (and it'sits Code property) is critical to the operation.

  1. Was creating an Airport class or struct a good solution in general, or am I solving the wrong problem / solving it the wrong way by creating the type? If it's not a good solution, what is a better solution?

  2. How should my application handle instances where the default(Airport) is used? A type of default(Airport) is nonsensical to my application, so I've been doing if (airport == default(Airport) { throw ... } in places where getting an instance of Airport (and it's Code property) is critical to the operation.

  1. Was creating an Airport class or struct a good solution in general, or am I solving the wrong problem / solving it the wrong way by creating the type? If it's not a good solution, what is a better solution?

  2. How should my application handle instances where the default(Airport) is used? A type of default(Airport) is nonsensical to my application, so I've been doing if (airport == default(Airport) { throw ... } in places where getting an instance of Airport (and its Code property) is critical to the operation.

1
source | link

struct with nonsensical default value

In my system I frequently operate with airport codes ("YYZ", "LAX", "SFO", etc.), they are always in the exact same format (3 letter, represented as uppercase). The system typically deals with 25-50 of these (different) codes per API request, with over a thousand allocations total, they are passed around through many layers of our application, and are compared for equality quite often.

We started with just passing strings around, which worked fine for a bit but we quickly noticed lots of programming mistakes by passing in a wrong code somewhere the 3 digit code was expected. We also ran into issues where we were supposed to do a case-insensitive comparison and instead did not, resulting in bugs.

From this, I decided to stop passing strings around and create an Airport class, which has a single constructor that takes and validates the airport code.

public sealed class Airport
{
    public Airport(string code)
    {
        if (code == null)
        {
            throw new ArgumentNullException(nameof(code));
        }

        if (code.Length != 3 || !char.IsLetter(code[0]) 
        || !char.IsLetter(code[1]) || !char.IsLetter(code[2]))
        {
            throw new ArgumentException(
                "Must be a 3 letter airport code.", 
                nameof(code));
        }

        Code = code.ToUpperInvariant();
    }

    public string Code { get; }

    public override string ToString()
    {
        return Code;
    }

    private bool Equals(Airport other)
    {
        return string.Equals(Code, other.Code);
    }

    public override bool Equals(object obj)
    {
        return obj is Airport airport && Equals(airport);
    }

    public override int GetHashCode()
    {
        return Code?.GetHashCode() ?? 0;
    }

    public static bool operator ==(Airport left, Airport right)
    {
        return Equals(left, right);
    }

    public static bool operator !=(Airport left, Airport right)
    {
        return !Equals(left, right);
    }
}

This made our code much easier to understand and we simplified our equality checks, dictionary / set usages. We now know that if our methods accept an Airport instance that it will behave the way we expect, it has simplified our method checks to a null reference check.

The thing I did notice, however, was the garbage collection was running much more often, which I tracked down to a lot of instances of Airport getting collected.

My solution to this was to convert the class into a struct. Mostly it was just a keyword change, with the exception of GetHashCode and ToString:

public override string ToString()
{
    return Code ?? string.Empty;
}

public override int GetHashCode()
{
    return Code?.GetHashCode() ?? 0;
}

To handle the case where default(Airport) is used.

My questions:

  1. Was creating an Airport class or struct a good solution in general, or am I solving the wrong problem / solving it the wrong way by creating the type? If it's not a good solution, what is a better solution?

  2. How should my application handle instances where the default(Airport) is used? A type of default(Airport) is nonsensical to my application, so I've been doing if (airport == default(Airport) { throw ... } in places where getting an instance of Airport (and it's Code property) is critical to the operation.

Notes: I reviewed the questions C#/VB struct – how to avoid case with zero default values, which is considered invalid for given structure?, and Use struct or not before asking my question, however I think my questions are different enough to warrant its own post.