структура с бессмысленным значением по умолчанию

12

В моей системе я часто работаю с кодами аэропортов ( "YYZ", "LAX", "SFO"и т.д.), они всегда находятся в том же формате (3 письма, представленный в верхнем регистре). Система обычно имеет дело с 25-50 из этих (разных) кодов на запрос API, с общим количеством распределений более тысячи, они проходят через многие уровни нашего приложения и довольно часто сравниваются на равенство.

Мы начали с простой передачи строк, которые немного подействовали, но мы быстро заметили множество ошибок в программировании, передав неправильный код, где ожидался трехзначный код. Мы также столкнулись с проблемами, при которых мы должны были делать сравнение без учета регистра, а не делать этого, что приводило к ошибкам.

Исходя из этого, я решил прекратить передачу строк и создать Airportкласс, который имеет единственный конструктор, который принимает и проверяет код аэропорта.

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);
    }
}

Это сделало наш код намного проще для понимания, и мы упростили наши проверки на равенство, использование словаря / набора. Теперь мы знаем, что если наши методы принимают Airportэкземпляр, который будет вести себя так, как мы ожидаем, он упростит наши проверки методов до проверки нулевых ссылок.

Однако я заметил, что сборка мусора выполнялась гораздо чаще, и я проследил это до многих случаев Airportсбора.

Мое решение для этого было преобразовать classв struct. В основном это было просто изменение ключевого слова, за исключением GetHashCodeи ToString:

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

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

Для обработки случая, где default(Airport)используется.

Мои вопросы:

  1. Было ли создание Airportкласса или структуры хорошим решением в целом, или я решил неправильную проблему / решил ее неправильно, создав тип? Если это не хорошее решение, что является лучшим решением?

  2. Как мое приложение должно обрабатывать случаи, когда default(Airport)используется? Тип не default(Airport)имеет смысла для моего приложения, поэтому я занимался if (airport == default(Airport) { throw ... }там, где получение экземпляра Airport(и его Codeсвойства) имеет решающее значение для операции.

Примечания: я рассмотрел вопросы C # / VB struct - как избежать случая с нулевыми значениями по умолчанию, который считается недействительным для данной структуры? , и используйте struct или нет, прежде чем задавать мой вопрос, однако я думаю, что мои вопросы достаточно разные, чтобы гарантировать свой собственный пост.

Мэтью
источник
7
Есть ли у мусора есть материальное воздействие на пути ваши преформ приложений? Другими словами, имеет ли это значение?
Роберт Харви,
В любом случае, да, классовое решение было «хорошим». То, как вы знаете, это решило вашу проблему, не создавая новых.
Роберт Харви
2
Один из способов решить эту default(Airport)проблему - просто запретить экземпляры по умолчанию. Вы можете сделать это, написав конструктор без параметров и бросив InvalidOperationExceptionили NotImplementedExceptionв нем.
Роберт Харви
3
Кстати, вместо того, чтобы подтвердить, что ваша строка инициализации на самом деле состоит из 3 буквенных символов, почему бы не сравнить ее с конечным списком всех кодов аэропортов (например, github.com/datasets/airport-codes или аналогичных)?
Дэн Пичельман
2
Я готов поспорить, что это не является корнем проблемы с производительностью. Обычный ноутбук может выделить порядка 10 миллионов объектов в секунду.
Эсбен Сков Педерсен

Ответы:

6

Обновление: я переписал свой ответ, чтобы устранить некоторые неверные предположения о структурах C #, а также ОП, сообщающий нам в комментариях об использовании интернированных строк.


Если вы можете контролировать данные, поступающие в вашу систему, используйте класс, который вы опубликовали в своем вопросе. Если кто-то бежит, default(Airport)он получит nullзначение обратно. Обязательно напишите свой приватный Equalsметод, который будет возвращать false при сравнении нулевых объектов Airport, а затем разрешите им NullReferenceExceptionлетать в других местах кода.

Однако, если вы берете данные в систему из источников, которые вы не контролируете, вам не нужно прерывать весь поток. В этом случае структура идеально подходит для простого факта, default(Airport)который даст вам нечто иное, чем nullуказатель. Создайте очевидное значение для представления «нет значения» или «значение по умолчанию», чтобы у вас было что печатать на экране или в файле журнала (например, «---»). На самом деле, я бы просто держал в codeсекрете и не выставлял Codeсобственность вообще - просто сосредоточился бы здесь на поведении.

public struct Airport
{
    private string code;

    public Airport(string code)
    {
        // Check `code` for validity, throw exceptions if not valid

        this.code = code;
    }

    public override string ToString()
    {
        return code ?? (code = "---");
    }

    // int GetHashcode()

    // bool Equals(...)

    // bool operator ==(...)

    // bool operator !=(...)

    private bool Equals(Airport other)
    {
        if (other == null)
            // Even if this method is private, guard against null pointers
            return false;

        if (ToString() == "---" || other.ToString() == "---")
            // "Default" values should never match anything, even themselves
            return false;

        // Do a case insensitive comparison to enforce logic that airport
        // codes are not case sensitive
        return string.Equals(
            ToString(),
            other.ToString(),
            StringComparison.InvariantCultureIgnoreCase);
    }
}

В худшем случае преобразование default(Airport)в строку выводит на печать "---"и возвращает ложь по сравнению с другими действующими кодами аэропорта. Любой код аэропорта по умолчанию не соответствует ничему, включая другие коды аэропорта по умолчанию.

Да, структуры должны быть значениями, выделенными в стеке, и любые указатели на динамическую память в основном сводят на нет преимущества производительности структур, но в этом случае значение структуры по умолчанию имеет значение и обеспечивает некоторое дополнительное пулевое сопротивление для остальных элементов. заявление.

Из-за этого я бы немного изменил правила.


Оригинальный ответ (с некоторыми фактическими ошибками)

Если вы можете контролировать данные, поступающие в вашу систему, я бы поступил так, как предложил Роберт Харви в комментариях: создать конструктор без параметров и вызвать исключение при его вызове. Это предотвращает попадание недействительных данных в систему через default(Airport).

public Airport()
{
    throw new InvalidOperationException("...");
}

Однако, если вы берете данные в систему из источников, которые вы не контролируете, вам не нужно прерывать весь поток. В этом случае вы можете создать недействительный код аэропорта, но сделать его очевидной ошибкой. Для этого потребуется создать конструктор без параметров и установить что- Codeто вроде «---»:

public Airport()
{
    Code = "---";
}

Поскольку вы используете в stringкачестве кода, нет смысла использовать структуру. Структура распределяется в стеке только для Codeразмещения в качестве указателя на строку в динамической памяти, поэтому здесь нет разницы между классом и структурой.

Если вы измените код аэропорта на массив из 3 элементов, то структура будет полностью размещена в стеке. Даже тогда объем данных не так велик, чтобы что-то изменить.

Грег Бургардт
источник
Если бы мое приложение использовало интернированные строки для Codeсвойства, изменило бы это ваше обоснование относительно вашей точки нахождения строки в динамической памяти?
Матфея
@ Мэтью: использование класса дает вам проблемы с производительностью? Если нет, то подбросьте монетку, чтобы решить, какую использовать.
Грег Бургхардт
4
@ Мэтью: действительно важно, чтобы вы централизовали проблемную логику нормализации кодов и сравнений. После этого «класс против структуры» - это просто академическая дискуссия, пока вы не измерите достаточно большое влияние на производительность, чтобы оправдать дополнительное время разработчика для академической дискуссии.
Грег Бургхардт
1
Это правда, я не против время от времени проводить академические дискуссии, если это поможет мне создавать более информированные решения в будущем.
Матфея
@ Мэтью: Да, вы абсолютно правы. Они говорят: «Говорить дешево». Это, конечно, дешевле, чем не говорить и строить что-то плохо. :)
Грег Бургардт
13

Используйте шаблон Flyweight

Поскольку аэропорт, по сути, является неизменным, нет необходимости создавать более одного экземпляра какого-либо конкретного, скажем, SFO. Используйте Hashtable или аналогичный (заметьте, я парень из Java, а не C #, поэтому точные детали могут отличаться) для кэширования аэропортов при их создании. Перед созданием нового проверьте Hashtable. Вы никогда не освобождаете аэропорты, поэтому GC не нужно их освобождать.

Еще одно небольшое преимущество (по крайней мере, в Java, не уверенное в C #) заключается в том, что вам не нужно писать equals()метод, простое ==подойдет. То же самое для hashcode().

user949300
источник
3
Отличное использование шаблона flyweight.
Нил
2
Предполагая, что OP продолжает использовать структуру, а не класс, разве интернирование строк уже не обрабатывает многократно используемые строковые значения? Структуры уже живут в стеке, строки уже используются повторно, чтобы избежать дублирования значений в памяти. Какая дополнительная выгода была бы получена от модели веса в полете?
Флейтер
Что-то, чтобы остерегаться. Если аэропорт добавлен или удален, вы захотите создать его таким образом, чтобы обновлять этот статический список без перезапуска приложения или его повторного развертывания. Аэропорты не добавляются и не удаляются часто, но владельцы бизнеса имеют тенденцию немного расстраиваться, когда простое изменение становится таким сложным. «Разве я не могу просто добавить это куда-нибудь ?! Почему мы должны запланировать выпуск / перезапуск приложения и доставить неудобства нашим клиентам?» Но я тоже сначала подумал об использовании статического кеша.
Грег Бургхардт
@ Флатер Разумная точка. Я бы сказал, что младшим программистам нужно меньше думать о стеке и куче. Плюс посмотрите мое дополнение - не нужно писать equals ().
user949300
1
@Greg Burghardt Если getAirportOrCreate()код правильно синхронизирован, нет никаких технических причин, по которым вы не можете создавать новые аэропорты по мере необходимости. Там могут быть деловые причины.
user949300
3

Я не особо продвинутый программист, но разве это не идеальное применение для Enum?

Существуют разные способы создания перечислимых классов из списков или строк. Вот тот, который я видел в прошлом, но не уверен, что это лучший способ.

https://blog.kloud.com.au/2016/06/17/converting-webconfig-values-into-enum-or-list/

Адам Б
источник
2
Когда есть потенциально тысячи различных значений (как в случае с кодами аэропортов), перечисление просто не практично.
Бен Коттрелл
Да, но ссылка, которую я разместил - как загружать строки как перечисления. Вот еще одна ссылка для загрузки таблицы поиска в виде enum. Это может быть немного работы, но будет использовать силу перечислений. exceptionnotfound.net/…
Адам Б
1
Или список допустимых кодов может быть загружен из базы данных или файла. Затем код аэропорта просто проверяется, чтобы быть в этом списке. Это то, что вы обычно делаете, когда больше не хотите жестко кодировать значения и / или список становится длинным для управления.
Нил
@BenCottrell - это именно то, для чего предназначены шаблоны gen и T4.
RubberDuck
3

Одна из причин, по которой вы наблюдаете больше активности GC, заключается в том, что вы сейчас создаете вторую строку - .ToUpperInvariant()версию исходной строки. Исходная строка имеет право на сборщик мусора сразу после запуска конструктора, а вторая - одновременно с Airportобъектом. Вы можете минимизировать его другим способом (обратите внимание на третий параметр string.Equals()):

public sealed class Airport : IEquatable<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;
    }

    public string Code { get; }

    public override string ToString()
    {
        return Code; // TODO: Upper-case it here if you really need to for display.
    }

    public bool Equals(Airport other)
    {
        return string.Equals(Code, other?.Code, StringComparison.InvariantCultureIgnoreCase);
    }

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

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

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

    public static bool operator !=(Airport left, Airport right)
    {
        return !Equals(left, right);
    }
}
Джесси С. Слайсер
источник
Разве это не приводит к разным хэш-кодам для одинаковых (но по-разному прописных) аэропортов?
Герой Бродит
Да, я так себе представляю. Dangit.
Джесси С. Слайсер
Это очень хороший момент, никогда не думал об этом, я посмотрю на внесение этих изменений.
Мэтью
1
Относительно GetHashCode, должен только использовать StringComparer.OrdinalIgnoreCase.GetHashCode(Code)или подобный
Мэтью