Хорошая или плохая практика? Инициализация объектов в геттере

167

Кажется, у меня странная привычка ... по крайней мере, по словам моего сотрудника. Мы вместе работали над небольшим проектом. Я написал классы так (упрощенный пример):

[Serializable()]
public class Foo
{
    public Foo()
    { }

    private Bar _bar;

    public Bar Bar
    {
        get
        {
            if (_bar == null)
                _bar = new Bar();

            return _bar;
        }
        set { _bar = value; }
    }
}

Так что, в основном, я инициализирую любое поле только тогда, когда вызывается метод get, а поле все еще пустое. Я полагал, что это уменьшит перегрузку, не инициализируя никакие свойства, которые нигде не используются.

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

Есть ли какие-либо возражения против этой практики, кроме личных предпочтений?

ОБНОВЛЕНИЕ: я рассмотрел много различных мнений относительно этого вопроса, и я буду стоять на своем принятом ответе. Тем не менее, теперь я пришел к гораздо лучшему пониманию концепции, и я могу решить, когда использовать ее, а когда нет.

Минусы:

  • Проблемы безопасности потоков
  • Не подчиняется запросу "setter", когда переданное значение является нулевым
  • Микро-оптимизация
  • Обработка исключений должна происходить в конструкторе
  • Необходимо проверить на нулевое значение в коде класса

Плюсы:

  • Микро-оптимизация
  • Свойства никогда не возвращают ноль
  • Задержка или избежание загрузки "тяжелых" предметов

Большинство минусов не применимо к моей текущей библиотеке, однако мне придется проверить, действительно ли «микрооптимизации» вообще что-то оптимизируют.

ПОСЛЕДНЕЕ ОБНОВЛЕНИЕ:

Хорошо, я изменил свой ответ. Мой оригинальный вопрос был или нет, это хорошая привычка. И теперь я убежден, что это не так. Возможно, я все еще буду использовать его в некоторых частях моего текущего кода, но не безоговорочно и определенно не всегда. Так что я собираюсь потерять свою привычку и подумать об этом, прежде чем использовать его. Спасибо всем!

Джон Виллемс
источник
14
Это шаблон с отложенной загрузкой, он не дает вам замечательную выгоду, но все же это хорошая вещь, imho.
Machinarius
28
Ленивое создание имеет смысл, если вы оказываете ощутимое влияние на производительность, или если эти элементы используются редко и потребляют непомерный объем памяти, или если создание их экземпляра занимает много времени и требуется только по требованию. В любом случае, обязательно учтите проблемы безопасности потоков (ваш текущий код не соответствует действительности ) и подумайте об использовании предоставленного класса Lazy <T> .
Крис Синклер
10
Я думаю, что этот вопрос лучше подходит для codereview.stackexchange.com
Эдуардо Бритес
7
@PLB это не одноэлементный шаблон.
Колин Маккей
30
Я удивлен, что никто не упомянул серьезную ошибку с этим кодом. У вас есть публичная собственность, которую я могу установить снаружи. Если я установлю его в NULL, вы всегда создадите новый объект и проигнорируете мой доступ для установки. Это может быть очень серьезной ошибкой. Для частной собственности это может быть хорошо. Лично я не люблю делать такие преждевременные оптимизации. Добавляет сложности без дополнительной выгоды.
SolutionYogi

Ответы:

170

Здесь у вас есть наивная реализация "ленивой инициализации".

Короткий ответ:

Безусловно, ленивая инициализация не очень хорошая идея. У него есть свои места, но нужно учитывать, какое влияние оказывает это решение.

Предпосылки и объяснение:

Конкретная реализация:
давайте сначала посмотрим на ваш конкретный пример и почему я считаю его реализацию наивной:

  1. Это нарушает принцип наименьшего сюрприза (POLS) . Когда значение присваивается свойству, ожидается, что это значение будет возвращено. В вашей реализации это не относится к null:

    foo.Bar = null;
    Assert.Null(foo.Bar); // This will fail
  2. Это приводит к некоторым проблемам foo.Barс потоками : два вызывающих в разных потоках потенциально могут получить два разных экземпляра, Barи один из них будет без связи с Fooэкземпляром. Любые изменения, внесенные в этот Barэкземпляр, молча теряются.
    Это еще один случай нарушения ПОЛ. Когда осуществляется доступ только к сохраненному значению свойства, ожидается, что он будет поточно-ориентированным. Хотя вы можете утверждать, что этот класс просто не является потокобезопасным, включая получатель вашего свойства, вам придется правильно документировать это, поскольку это не является нормальным случаем. Кроме того, введение этой проблемы не является необходимым, как мы скоро увидим.

В общем:
сейчас пришло время взглянуть на ленивую инициализацию в целом:
ленивая инициализация обычно используется для задержки создания объектов, которые требуют много времени для создания или которые занимают много памяти после полной сборки.
Это очень веская причина для использования отложенной инициализации.

Однако такие свойства обычно не имеют сеттеров, что избавляет от первой проблемы, указанной выше.
Кроме того, Lazy<T>для избежания второй проблемы будет использоваться поточно-ориентированная реализация .

Даже при рассмотрении этих двух моментов в реализации ленивого свойства, следующие моменты являются общими проблемами этого шаблона:

  1. Строительство объекта может быть неудачным, что приведет к исключению из метода получения свойства. Это еще одно нарушение ПОЛ, и поэтому его следует избегать. Даже раздел о свойствах в «Руководстве по проектированию для разработки библиотек классов» прямо заявляет, что получатели свойств не должны генерировать исключения:

    Избегайте выбрасывания исключений от получателей собственности.

    Собственники должны быть простыми операциями без каких-либо предварительных условий. Если получатель может выдать исключение, рассмотрите возможность изменения свойства как метода.

  2. Вредна автоматическая оптимизация компилятором, а именно встраивание и предсказание переходов. Пожалуйста, смотрите ответ Билла К. для подробного объяснения.

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

Этот шаблон имеет свое место, но он не является общей передовой практикой при реализации классов. Он не должен использоваться безоговорочно по причинам, указанным выше.


В этом разделе я хочу обсудить некоторые моменты, которые другие выдвинули в качестве аргументов для безоговорочного использования ленивой инициализации:

  1. Сериализация:
    EricJ заявляет в одном комментарии:

    Объект, который может быть сериализован, не будет вызывать свой конструктор при десериализации (зависит от сериализатора, но многие обычные ведут себя так). Ввод кода инициализации в конструктор означает, что вы должны обеспечить дополнительную поддержку десериализации. Этот шаблон избегает этого специального кодирования.

    Есть несколько проблем с этим аргументом:

    1. Большинство объектов никогда не будут сериализованы. Добавление некоторой поддержки для него, когда оно не нужно, нарушает YAGNI .
    2. Когда класс должен поддерживать сериализацию, существуют способы включить его без обходного пути, который на первый взгляд не имеет ничего общего с сериализацией.
  2. Микрооптимизация. Ваш главный аргумент заключается в том, что вы хотите создавать объекты только тогда, когда кто-то действительно обращается к ним. Итак, вы на самом деле говорите об оптимизации использования памяти.
    Я не согласен с этим аргументом по следующим причинам:

    1. В большинстве случаев еще несколько объектов в памяти ни на что не влияют. Современные компьютеры имеют достаточно памяти. Без фактических проблем, подтвержденных профилировщиком, это преждевременная оптимизация, и на это есть веские причины.
    2. Я признаю тот факт, что иногда такая оптимизация оправдана. Но даже в этих случаях ленивая инициализация не кажется правильным решением. На это есть две причины:

      1. Ленивая инициализация потенциально снижает производительность. Может быть, только незначительно, но, как показал ответ Билла, влияние будет больше, чем можно подумать на первый взгляд. Таким образом, этот подход в основном соотносит производительность с памятью.
      2. Если у вас есть проект, в котором распространено использование только частей класса, это намекает на проблему с самим дизайном: рассматриваемый класс, скорее всего, несет более одной ответственности. Решение было бы разделить класс на несколько более сфокусированных классов.
Дэниэл Хилгарт
источник
4
@JohnWillemse: Это проблема вашей архитектуры. Вы должны реорганизовать ваши классы таким образом, чтобы они были меньше и более сфокусированы. Не создавайте один класс для 5 разных вещей / задач. Создайте 5 классов вместо.
Даниэль Хилгарт,
26
@JohnWillemse Возможно, рассмотрим это как случай преждевременной оптимизации. Если у вас нет узких мест в измерении производительности / памяти, я бы порекомендовал это, поскольку это увеличивает сложность и создает проблемы с многопоточностью.
Крис Синклер
2
+1, это не очень хороший выбор дизайна для 95% классов. Ленивая инициализация имеет свои плюсы, но не должна быть обобщена для ВСЕХ свойств. Это добавляет сложность, сложность чтения кода, проблемы безопасности потоков ... без ощутимой оптимизации в 99% случаев. Кроме того, как SolutionYogi сказал в качестве комментария, код OP содержит ошибки, что доказывает, что этот шаблон не тривиален для реализации и его следует избегать, если в действительности не требуется ленивая инициализация.
ken2k
2
@DanielHilgarth Спасибо за то, что вы прошли весь путь, чтобы записать (почти) все неправильно с использованием этого шаблона безоговорочно. Прекрасная работа!
Алекс
1
@DanielHilgarth Ну да и нет. Нарушение является проблемой здесь, так что да. Но также «нет», потому что POLS - это строго принцип, по которому вы, вероятно, не будете удивлены кодом . Если Foo не был разоблачен вне вашей программы, вы можете рискнуть или нет. В этом случае я почти гарантирую, что вы в конечном итоге будете удивлены , потому что вы не контролируете способ доступа к свойствам. Риск просто стал ошибкой, и ваш аргумент о nullделе стал намного сильнее. :-)
atlaste
49

Это хороший выбор дизайна. Настоятельно рекомендуется для библиотечного кода или основных классов.

Он вызывается некоторой «отложенной инициализацией» или «отложенной инициализацией» и, как правило, считается хорошим выбором при проектировании.

Во-первых, если вы инициализируете в объявлении переменных уровня класса или конструктора, то когда ваш объект создается, у вас есть накладные расходы на создание ресурса, который никогда не будет использоваться.

Во-вторых, ресурс создается только при необходимости.

В-третьих, вы избегаете сбора мусора неиспользованным объектом.

Наконец, легче обрабатывать исключения инициализации, которые могут возникать в свойстве, чем исключения, возникающие при инициализации переменных уровня класса или конструктора.

Есть исключения из этого правила.

Что касается аргумента производительности дополнительной проверки инициализации в свойстве get, то он незначителен. Инициализация и удаление объекта являются более значительным ударом по производительности, чем простая проверка нулевого указателя с помощью прыжка.

Рекомендации по разработке библиотек классов по адресу http://msdn.microsoft.com/en-US/library/vstudio/ms229042.aspx

относительно Lazy<T>

Универсальный Lazy<T>класс был создан именно для того, что хочет плакат, см. Ленивая инициализация на http://msdn.microsoft.com/en-us/library/dd997286(v=vs.100).aspx . Если у вас более старые версии .NET, вы должны использовать шаблон кода, показанный в вопросе. Этот шаблон кода стал настолько распространенным, что Microsoft посчитала необходимым включить класс в новейшие библиотеки .NET, чтобы упростить реализацию шаблона. Кроме того, если вашей реализации нужна безопасность потоков, вы должны добавить ее.

Примитивные типы данных и простые классы

Obvioulsy, вы не собираетесь использовать ленивую инициализацию для примитивного типа данных или простого использования класса, как List<string>.

Прежде чем комментировать ленивый

Lazy<T> была введена в .NET 4.0, поэтому, пожалуйста, не добавляйте еще один комментарий относительно этого класса.

Прежде чем комментировать о микрооптимизации

Когда вы создаете библиотеки, вы должны учитывать все оптимизации. Например, в классах .NET вы увидите битовые массивы, используемые для переменных логического класса по всему коду для уменьшения потребления памяти и фрагментации памяти, просто чтобы назвать две «микрооптимизации».

Что касается пользовательских интерфейсов

Вы не собираетесь использовать отложенную инициализацию для классов, которые непосредственно используются пользовательским интерфейсом. На прошлой неделе я потратил большую часть дня, снимая ленивую загрузку восьми коллекций, используемых в модели представления для комбо-боксов. У меня есть, LookupManagerкоторый обрабатывает ленивую загрузку и кэширование коллекций, необходимых любому элементу пользовательского интерфейса.

«сеттер»

Я никогда не использовал set-property ("setters") для каких-либо отложенных загруженных свойств. Поэтому вы никогда не позволите foo.Bar = null;. Если вам нужно установить, Barто я бы создал метод с именем SetBar(Bar value)и не использовать ленивую инициализацию

Коллекции

Свойства коллекции классов всегда инициализируются при объявлении, потому что они никогда не должны быть нулевыми.

Сложные классы

Позвольте мне повторить это по-другому, вы используете ленивую инициализацию для сложных классов. Которые обычно плохо спланированы.

наконец

Я никогда не говорил делать это для всех классов или во всех случаях. Это плохая привычка.

AMissico
источник
6
Если вы можете вызывать foo.Bar несколько раз в разных потоках без какого-либо промежуточного значения, но получать разные значения, тогда у вас паршивый плохой класс.
Ли Райан
25
Я думаю, что это плохое эмпирическое правило без многих соображений. Если Бар не является известным источником ресурсов, это ненужная микрооптимизация. А в случае, когда Bar требует много ресурсов, в .net встроен потокобезопасный Lazy <T>.
Эндрю Хэнлон
10
«легче обрабатывать исключения инициализации, которые могут возникать в свойстве, чем исключения, возникающие при инициализации переменных уровня класса или конструктора». - Ладно, это глупо. Если объект не может быть инициализирован по какой-либо причине, я хочу знать как можно скорее. Т.е. сразу, когда он будет построен. Существуют веские аргументы в пользу использования отложенной инициализации, но я не думаю, что это хорошая идея использовать его повсеместно .
миллимус
20
Я действительно волнуюсь, что другие разработчики увидят этот ответ и думают, что это действительно хорошая практика (о мальчик). Это очень плохая практика, если вы используете ее безоговорочно. Помимо того, что уже было сказано, вы делаете жизнь каждого человека намного сложнее (для разработчиков клиентов и для разработчиков технического обслуживания) за столь малую выгоду (если вообще выигрываете). Вы должны услышать от профессионалов: Дональд Кнут из серии «Искусство компьютерного программирования» сказал, что «преждевременная оптимизация - корень всего зла». То, что вы делаете, не только зло, это дьявольское!
Алекс
4
Есть так много показателей, вы выбираете неправильный ответ (и неправильное решение программирования). В вашем списке больше минусов, чем плюсов. У вас больше людей ругается против этого, чем за это. Более опытные участники этого сайта (@BillK и @DanielHilgarth), которые разместили в этом вопросе, против этого. Ваш коллега уже сказал вам, что это неправильно. Серьезно, это неправильно! Если я поймаю на этом одного из разработчиков моей команды (я являюсь руководителем команды), он возьмет 5-минутный таймаут, а затем будет рассказано, почему он никогда не должен этого делать.
Алекс
17

Считаете ли вы реализацию такого шаблона с помощью Lazy<T>?

В дополнение к простому созданию ленивых загруженных объектов, вы получаете безопасность потока, пока объект инициализируется:

Как говорили другие, вы лениво загружаете объекты, если они действительно ресурсоемки или для их загрузки требуется некоторое время во время создания объекта.

Матиас Фидемрайзер
источник
Спасибо, я понимаю это сейчас, и я определенно буду изучать Lazy<T>сейчас и воздержусь от использования, как я всегда делал
Джон Виллемс
1
Вы не получаете безопасность волшебной нити ... Вы все еще должны думать об этом. Из MSDN:Making the Lazy<T> object thread safe does not protect the lazily initialized object. If multiple threads can access the lazily initialized object, you must make its properties and methods safe for multithreaded access.
Эрик Дж.
@EricJ. Конечно, конечно. Вы получаете безопасность потока только при инициализации объекта, но позже вам нужно иметь дело с синхронизацией, как с любым другим объектом.
Матиас Фидемрайзер
9

Я думаю, это зависит от того, что вы инициализируете. Я, вероятно, не сделал бы это для списка, так как стоимость строительства довольно мала, поэтому он может идти в конструкторе. Но если бы это был предварительно заполненный список, то, вероятно, я бы не стал, пока он не понадобился впервые.

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

Колин Маккей
источник
Спасибо! В этом есть смысл.
Джон Виллемс
9

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

Луис Теллез
источник
Я не думаю, что это обратная сторона.
Питер Порфи
Почему это оборотная сторона? Просто сверьтесь с любым вместо нуля. if (! Foo.Bars.Any ())
s.meijer
6
@PeterPorfy: это нарушает ПОЛ . Вы вставляете null, но не возвращаете это. Обычно вы предполагаете, что получите то же значение, которое вы указали в свойстве.
Даниэль Хилгарт,
@DanielHilgarth Еще раз спасибо. Это очень веский аргумент, который я раньше не рассматривал.
Джон Виллемс
6
@AMissico: Это не выдуманная концепция. Подобно тому, как нажатие кнопки рядом с входной дверью должно звенеть в дверной звонок, вещи, которые выглядят как свойства, должны вести себя как свойства. Открытие люка под ногами - удивительное поведение, особенно если кнопка не помечена как таковая.
Брайан Бетчер
8

Ленивая инстанциация / инициализация - вполне жизнеспособный паттерн. Имейте в виду, однако, что, как правило, потребители вашего API не ожидают, что методы получения и установки получат заметное время от POV конечного пользователя (или потерпят неудачу).

Tormod
источник
1
Я согласен, и я немного отредактировал свой вопрос. Я ожидаю, что полная цепочка базовых конструкторов займет больше времени, чем создание экземпляров классов, только когда они необходимы.
Джон Виллемс
8

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

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

Одна из лучших вещей об объекте - то, что он предлагает безопасную, доверенную среду. Самый лучший случай, если вы сделаете как можно больше полей "Final", заполнив их все конструктором. Это делает ваш класс довольно пуленепробиваемым. Позволять менять поля через сеттеры немного меньше, но не страшно. Например:

класс SafeClass
{
    String name = "";
    Целочисленный возраст = 0;

    public void setName (String newName)
    {
        assert (newName! = null)
        имя = NEWNAME;
    } // следуем этой схеме для возраста
    ...
    public String toString () {
        Строка s = "Безопасный класс имеет имя:" + name + "и возраст:" + age
    }
}

С вашим шаблоном метод toString будет выглядеть так:

    если (имя == ноль)
        выдать новое IllegalStateException («SafeClass вошел в недопустимое состояние! имя равно нулю»)
    если (возраст == ноль)
        выдать новое IllegalStateException («SafeClass попал в недопустимое состояние! age is null»)

    public String toString () {
        Строка s = "Безопасный класс имеет имя:" + name + "и возраст:" + age
    }

Не только это, но вам нужны нулевые проверки везде, где вы, возможно, используете этот объект в своем классе (Вне класса ваш класс безопасен из-за нулевой проверки в получателе, но вы должны в основном использовать членов классов внутри класса)

Кроме того, ваш класс постоянно находится в неопределенном состоянии - например, если вы решили сделать этот класс спящим, добавив несколько аннотаций, как бы вы это сделали?

Если вы принимаете какое-либо решение на основе микрооптомизации без требований и тестирования, это почти наверняка будет неправильным решением. На самом деле, есть действительно очень хороший шанс, что ваш паттерн на самом деле замедляет работу системы даже в самых идеальных условиях, потому что оператор if может вызвать сбой предсказания ветвления в ЦП, который будет тормозить вещи намного, намного, намного больше, чем просто присваивая значение в конструкторе, если создаваемый вами объект не является достаточно сложным или не поступает из удаленного источника данных.

В качестве примера проблемы прогнозирования Brance (которую вы повторяете много раз, чаще всего один раз), смотрите первый ответ на этот удивительный вопрос: почему быстрее обрабатывать отсортированный массив, чем несортированный массив?

Билл К
источник
Спасибо за ваш вклад. В моем случае ни один из классов не имеет методов, которые могут нуждаться в проверке на нулевое значение, так что это не проблема. Я приму к сведению ваши другие возражения.
Джон Виллемс
Я не очень понимаю это. Это означает, что вы не используете своих членов в классах, где они хранятся, - что вы просто используете классы в качестве структур данных. Если это так, вы можете прочитать javaworld.com/javaworld/jw-01-2004/jw-0102-toolbox.html, в котором есть хорошее описание того, как можно улучшить ваш код, избегая внешнего манипулирования состоянием объекта. Если вы манипулируете ими внутренне, как вы можете это сделать без повторной нулевой проверки?
Билл К
Части этого ответа хороши, но части кажутся надуманными. Обычно при использовании этого шаблона, toString()будет вызывать getName(), а не использовать nameнапрямую.
Изката
@BillK Да, классы - это огромная структура данных. Вся работа выполняется в статических классах. Я проверю статью по ссылке. Спасибо!
Джон Виллемс
1
@izkata На самом деле, внутри класса кажется, что вопрос о том, используете ли вы геттер или нет, в большинстве мест, где я работал, использовал член напрямую. Кроме того, если вы всегда используете метод получения, метод if () еще более вреден, потому что сбои прогнозирования ветвления будут происходить чаще И из-за ветвления среда выполнения, скорее всего, будет иметь больше проблем при заполнении метода получения. Это все спорно, однако, с откровением Иоанна о том , что они являются структуры данных и статические классы, то есть вещь , которую я бы больше всего касается.
Билл К
4

Позвольте мне добавить еще один момент ко многим хорошим моментам, высказанным другими ...

Отладчик будет ( по умолчанию ) оценивать свойства при пошаговом выполнении кода, что потенциально может привести к созданию экземпляра Barбыстрее, чем обычно, если просто выполнить код. Другими словами, простое действие отладки изменяет выполнение программы.

Это может или не может быть проблемой (в зависимости от побочных эффектов), но это то, что нужно знать.

Бранко Димитриевич
источник
2

Вы уверены, что Фу вообще должен что-то создавать?

Мне кажется вонючим (хотя и не обязательно неправильным ) позволять Фу создавать что-либо вообще. Если Foo не является явной целью быть фабрикой, он не должен создавать экземпляры своих собственных сотрудников, а вместо этого вводить их в свой конструктор .

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

KaptajnKold
источник
4
@ BenjaminGruenbaum Нет, не совсем. И с уважением, даже если бы это было, какой момент вы пытались сделать?
KaptajnKold