31

Коллеги, я не вполне понимаю одну из рекомендаций в .NET design guidelines.

В ней говорится:

DO prefer protected accessibility over public accessibility for virtual members. Public members should provide extensibility (if required) by calling into a protected virtual member.

The public members of a class should provide the right set of functionality for direct consumers of that class. Virtual members are designed to be overridden in subclasses, and protected accessibility is a great way to scope all virtual extensibility points to where they can be used.

то есть

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

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

Прочитав этот текст, я всё же не понимаю, какая проблема может быть в том, если семейство виртуальных функций будет объявлено открытым. Например, вот в таком коде:

class Human : IDisposable
{
    IDisposable property = new Property();
    public virtual Dispose()
    {
        property.Dispose();
    }
}

class Spy : Human
{
    IDisposable spyGadgets = new SpyGadgets();
    public virtual Dispose()
    {
        base.Dispose();
        spyGadgets.Dispose();
    }
}

Какие могут быть проблемы с таким кодом? О чём меня пытается предупредить документация? Если с этим случаем всё хорошо, то в каком случае возможны проблемы?

Приведите, если можно, осмысленный пример с кодом (не с классами Foo и Bar).


Огромное спасибо за ответы! Мне было нелегко выбрать, какой из них отметить галочкой, потому что все ответы очень хороши, и проливают свет на проблему с разных сторон.

7
  • Возникает другой вопрос, рекомендует ли этот гайд объявлять публичные методы final(sealed)?
    – vp_arth
    25 ноя 2017 в 11:04
  • @vp_arth: Ну, если делать согласно этому гайду, то виртуальные методы не будут открытыми. А на невиртуальных sealed по идее не нужен.
    – VladD
    25 ноя 2017 в 11:08
  • Хм, а как в c# защищаются от переопределения невиртуальных методов? public new ....() {}
    – vp_arth
    25 ноя 2017 в 11:25
  • @vp_arth: Никак?
    – VladD
    25 ноя 2017 в 11:25
  • 4
    Я сплю и мне снится, что VladD задает вопрос... :D
    – andreycha
    25 ноя 2017 в 19:40

6 ответов 6

21

Это же рекомендации для разработчиков фреймворков. Очевидно, что разработчики фреймворков будут выпускать новые версии своих фреймворков. Также понятно, что одной из важнейших задач для них - сохранение обратной совместимости, по мере возможностей. А значит, что клиентов, что используют их код, следует максимально ограничить. То есть, это мы (ну или только я такой рукожоп) привыкли писать классы наследуемыми - но если мы пишем фреймворк, то нужна достаточно веская причина сделать класс наследуемым. Также нужна веская причина сделать метод виртуальным. Но вот ты сделал публичный метод виртуальным, и теперь клиенты могут наследоваться, перегрузить метод и запускать сами написанный ими же код, используя наш фреймвок - и мы уже не можем это контролировать. Я к тому, что делая публичный метод виртуальным, мы даем право клиенту решать, что будет делать наш АПИ и мы уже никак не можем ничего изменить, не сломав обратную совместимость. Однако, сделав защищенный метод виртуальным, мы не гарантируем клиенту, что этот метод не перестанет использоваться в будущем, если наш публичный АПИ будет изменен. Таким образом клиенты, что перегрузили защищенный метод, сохраняют обратную совместимость, даже если логика публичных методов была изменена.

Походу надо добавить пример, хотя я и не мастер примеров :) Допустим есть следующие классы:

public class CsvWriter1<T>
{
    protected void WriteHeader(TextWriter stream);
    protected void WriteBody(IEnumerable<T> obj, TextWriter stream);

    protected virtual void WriteInternal(IEnumerable<T> obj, TextWriter stream)
    {
        WriteHeader(stream);
        WriteBody(obj, stream);
    }

    public void Write(IEnumerable<T> obj, TextWriter stream)
    {
        WriteInternal(obj, stream);
    }
}


public class CsvWriter2<T>
{
    protected void WriteHeader(TextWriter stream);
    protected void WriteBody(IEnumerable<T> obj, TextWriter stream);

    public virtual void Write(IEnumerable<T> obj, TextWriter stream)
    {
        WriteHeader(stream);
        WriteBody(obj, stream);
    }
}

Теперь представим, что в следующей версии нашего чудесного фреймворка нам надо писать объекты в CSV без заголовка. То есть, заголовок больше не нужен. И переопределять это больше нельзя. Что делать? Для класса CsvWriter1 все просто

public class CsvWriter1<T>
{
    protected void WriteHeader(TextWriter stream);
    protected void WriteBody(IEnumerable<T> obj, TextWriter stream);

    protected virtual void WriteInternal(IEnumerable<T> obj, TextWriter stream)
    {
        WriteHeader(stream);
        WriteBody(obj, stream);
    }

    protected void WriteInternalNew(IEnumerable<T> obj, TextWriter stream)
    {
        WriteBody(obj, stream);
    }

    public void Write(IEnumerable<T> obj, TextWriter stream)
    {
        WriteInternalNew(obj, stream);
    }
}

В случае с классом CsvWriter2 мы в луже. Так как указанное изменение для него будет обрано несовместимым, и конечно сломает логику клиентских классов.

3
  • Ага, то есть юзкейс, например, такой: мы выпустили фреймворк, и теперь хотим поменять логику, допустим, сделать логирование вызовов публичного метода, или там кеширование результатов. Если этот публичный метод виртуальный, то у нас нету контроля. А если он невиртуальный, то всё в наших руках, но виртуальная точка расширения есть, хотя и в другом месте. Правильно?
    – VladD
    25 ноя 2017 в 19:28
  • Да. Если утрировать - мы выпустили класс и сделали логирование виртуальным методом. А в следующей версии решили отказаться от логирования совсем. В случае защищенного метода мы просто перестаем его использовать в публичных методах в коде, он остается только для поддержки обратной совместимости. Как итог, наш АПИ не сломан, пубичные методы невиртуальные, все хорошо. В случае публичного метода, он по прежнему может быть использован клиентом в его коде, что может привести к непонятным ситуациям.
    – tym32167
    25 ноя 2017 в 19:38
  • @VladD я не мастер примеров и объяснений, добавил апдейт, пояснил как смог :)
    – tym32167
    25 ноя 2017 в 19:51
17

Впервые идею невиртуальных интерфейсов я встретил у Герба Саттера еще в начале нулевых, и тогда мне это казалось отличной идеей (что не удивительно, я писал на С++ и там эта практика была весьма популярна в принципе, да еще и описана светилами индустрии).

И вот его же (Саттера) статья - Virtuality - с объяснением этой темы (найдено в том самом посте, на который я ссылаюсь в предыдущем абзаце):

Traditionally, many programmers were used to writing base classes using public virtual functions to directly and simultaneously specify both the interface and the customizable behavior. For example, we might write:

// Example 1: A traditional base class.
//
class Widget
{
public:
  // Each of these functions might optionally be
  // pure virtual, and if so might or might not have
  // an implementation in Widget; see Item 27 in [1].
  //
  virtual int Process( Gadget& );
  virtual bool IsDone();
  // ...
};

The problem is that "simultaneously" part, because each virtual function is doing two jobs: It's specifying interface because it's public and therefore directly part of the interface Widget presents to the rest of the world; and it's specifying implementation detail, namely the internally customizable behavior, because it's virtual and therefore provides a hook for derived classes to replace the base implementation of that function (if any). That a public virtual function inherently has two significantly different jobs is a sign that it's not separating concerns well and that we should consider a different approach.

Когда я перешел в .NET лет 10 назад, я старался пользоваться этой идиомой, просто по привычке. Но потом, начал на нее забивать.

Теперь, можно поанализировать этот совет.

  1. Советом практически никто не пользуется в мире .NET-а. Даже .NET Framework забил на это с большой колокольни.
  2. Совет разумен, если рассматривать его, как частный случай Шаблонного метода. Если в базовом классе определено какое-то поведение: валидация аргументов, базовая фунциональность, хоть что-то. Тогда разделение невиртуального метода открытого интерфейса и защищенного метода, дополнияющего функционал, имеет смысл.

Выводы: со всем уважением к Framework Design Guidelines и к Саттеру лично, но на этот совет можно и нужно забить. Плодить методы ради того, чтобы плодить методы - это никуда не годное время препровождение. Я этим советом очень и очень давно не пользуюсь и не помню ни одной проблемы из-за этого. И да, этому же совету не следуют авторы фреймворка и тоже особых проблем не имеют (из-за этого).

Ув. @Denis Gladkiy задал такой вопрос:

А какие-нибудь аргументы против NVI по делу-то будут?

То на него стоит ответить более явны образом:

  1. NVI - это дополнительная работа, а значит, что не нужны аргументы "против". Нужны аргументы "за", ибо YAGNI и преждевременное обобщение.
  2. Я не видел в индустрии акцент на этот паттерн нигде, кроме С++. Мое предположение, что он появился в .NET, поскольку его авторы пришли из мира С++.
  3. В .NET Framework много примеров нарушеня этого принципа, начиная от System.Object, заканчивая классом Stream
  4. Этому правилу уже очень много лет, но новый АПИ, который появляется или расширяется в .NET Framework-е (тот же Stream.FlushAsync) все еще ему не следует. Если бы этот принцип был бы полезен, а не следование ему было бы плохим, но новый АПИ, который не связан обратной совместимостью уже бы ему следовал.
  5. Основываясь на собственном опыте или опыте коллег, я не вижу смысла в следовании этому паттерну, если только в невиртуальном методе есть какое-то поведение. Я понимаю, что мой опыт и опыт моих коллег - это субъективное мнение, но мне сложно представить ситуацию, когда нарушение этого принципа на самом деле приведет к серьезной проблеме.

Я задал этот же вопрос автору Framework Design Guidelines, Кжиштофу Квалине - "Что он думает по поводу этой рекомендации сегодня?". Вот его ответ.

If I was writing the guidance today, I would make it a “consider” guideline. When the guideline be was originally written, we had lots of Opportunistic (VB) customers asking us to simplify extensibility APIs, by for example hiding them from public surface area. Now most .NET developers don’t get scared by public virtual members.

20
  • Сергей, читал ваши некоторые материалы по юнит-тестированию, в частности по фреймворку Moq. Не могли бы взглянуть на связанный вопрос?
    – Anatol
    30 ноя 2017 в 5:14
  • 1
    А какие-нибудь аргументы против NVI по делу-то будут? А то результат ответа попахивает "аппеляцией к ваторитету": куча народу ринется применять, потому что "это ж сам Тепляков написал!!!11". "Можно и нужно забить", "я давно не пользуюсь и не помню проблем"... Или не видите проблем? Или у вас была конкретная ситуация не применимая в другом случае? То, что авторы фреймворка не имеют проблем, это они сами вам сказали? И опять же, может, они их имеют, но не видят? Конкретно у меня, из практики, например, несколько иная картина. 30 ноя 2017 в 7:03
  • 1
    @DenisGladkiy Обновил ответ. З.Ы. Будет интересно услышать Ваш опыт. З.Ы.Ы. Я Вам на палец наступил? Был груб? Что-то не так Вам лично написал? 30 ноя 2017 в 8:02
  • 1
    @tym32167 моя главная проблема с этим "подходом", что это именно пожелание, а не паттерн. У паттерна есть контекст, применимость, ограничения. Это просто "следуйте правилу будет"... что? Конечно мы найдем примеры невиртуальных интерфейсов, но это уже будет пример Шаблонного метода. Суть правил в том, что им всегда следуют. Суть паттернов в том, что им следуют, когда нужно. 30 ноя 2017 в 18:18
  • 1
    Ну если шаблонный метод публичный и невиртуальный, а шаги внутри protected virtual, то отличий никаких, разве что мотивация. То есть код то тот же самый, только причина, почему он так написан другая. Да и сам шаблонный метод же может быть и приватным или защищенным, что уже не относится к вопросу. То есть, если тянуть за уши, то можно саму рекомедацию дотянуть до частного случая шаблонного метода. Но это, имхо, демагогия о высоких материях. В общем, я понял, что мы вроде одно и то же говорим, только разными словами.
    – tym32167
    1 дек 2017 в 10:22
11

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

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

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

Поэтому да, "do prefer". Но именно prefer.

3
  • 1
    То есть тогда это правило должно включать не только «делайте виртуальные методы непубличными», но и вообще «не выносите в виртуальные методы что-то, что пользователь может сломать». То есть по сути, «помещайте в виртуальные методы как можно меньше логики — клиентам будет сложнее сломать фреймворк». Правильно?
    – VladD
    25 ноя 2017 в 20:39
  • И как бы публичные методы в этом отношении хуже всего т. к. они внешние, вызывают внутренние методы, и значит, в них больше всего логики.
    – VladD
    25 ноя 2017 в 20:41
  • @VladD можно сказать и так. Наследование -- это же всегда компромисс между переиспользованием/расширяемостью и связностью/возможным нарушением инкапсуляции. И если ты разрешаешь наследовать класс, нужно найти баланс между удобством наследования и возможностью выстрелить в ногу. P.S. Ну и все-таки это не правило, а рекомендация.
    – andreycha
    25 ноя 2017 в 22:04
7

С методом Dispose никаких проблем нет: его логика очень проста, а его контракт раз и навсегда задан Microsoft, и не изменится в будущем.

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

interface IValidator 
{
    ValidationResult Validate(object obj);
}

class TypedValidator<T> : IValidator
{
    public ValidationResult Validate(object obj)
    {
        if (obj is T t)
            return Validate(t);
        else
            return ValidationResult.Fail($"Wrong object type");
    }

    protected virtual ValidationResult Validate(T obj) => ValidationResult.Ok;
}

Если бы в примере выше именно открытый метод Validate был объявлен виртуальным - то любой производный класс вынужден был бы дублировать логику базового класса.

Более простой случай - защитные проверки аргументов. Аргументы достаточно проверить один раз, в базовом классе - не следует писать одну и ту же проверку в каждом классе.

class Foo
{
     public virtual void Baz(object obj)
     {
         if (obj == null) throw new ArgumentNullException("obj");

         // ...
     }
}

class Bar: Foo 
{
     public override void Baz(object obj)
     {
         if (obj == null) throw new ArgumentNullException("obj"); // Этой проверки могло бы не быть

         // ...

         base.Baz(obj);
     }
}
4
  • 3
    Окей, я понял ваш ответ. Но этот юзкейс должен был бы оформиться в «выносите общую функциональность в общие методы». То есть понятно, что если точка расширения внутри, то виртуальный метод не совпадает с нужным открытым интерфейсом класса, и само собой не должен быть открытым. И тогда это подпало бы под гораздо более общее правило «не делайте открытыми методы, которые юзер не должен дёргать». А что же тут специфического для виртуальных методов?
    – VladD
    25 ноя 2017 в 10:46
  • 2
    @VladD ну, авторы книг любят повторять одно и то же 10 раз, это известная проблема. 30 ноя 2017 в 9:41
  • Ага, понял. То есть известный юзкейс более общей проблемы, правильно?
    – VladD
    30 ноя 2017 в 10:01
  • @VladD ну да, мне так кажется 30 ноя 2017 в 10:02
5

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

Разумеется можно идти и обратным путем - последовательно выносить специфику в отдельный метод, но это обычно требует от меня больше внимания и ручной работы.

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

3
  • 1
    То есть я правильно понимаю — это такой метод защитного программирования на случай, если «точка расширения» с развитием класса переедет из публичной функции в что-то, что не должно быть публичной функцией? Допустим, что так. Но если мы при этом всё равно ломаем обратную совместимость (раз предусмотреть конечный дизайн вначале невозможно, и нам придётся менять сигнатуру виртуальной функции), то проще было ли сделать публичную функцию невиртуальной в этот момент? Всё равно потомки вынуждены будут обновить свой код.
    – VladD
    25 ноя 2017 в 12:47
  • 1
    @VladD В общем да, лично для меня это именно метод защитного программирования. А все предусмотреть все равно не получится, но если можно предусмотреть хотя бы что-то - уже хорошо.
    – rdorn
    25 ноя 2017 в 12:54
  • Понял, спасибо! Подумаю над этим.
    – VladD
    25 ноя 2017 в 13:03
4

Плюс пять копеек за NVI: NVI и C#

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

Как мне кажется, это сродни пониманию в необходимости делать бэкапы

Люди делятся на два типа - тех, кто уже делает бэкапы и тех, что ещё нет.

(Бэкапы по привычке уже делают многие, но тестируют свои бэкапы ещё не все, например) и многим другим подобным вещам.

Ваш ответ

By clicking “Отправить ответ”, you agree to our terms of service and acknowledge you have read our privacy policy.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.