Страницы

Поиск по вопросам

понедельник, 25 ноября 2019 г.

Нужно ли виртуальные методы объявлять как protected?


Коллеги, я не вполне понимаю одну из рекомендаций в .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 fo
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).



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


Ответы

Ответ 1



Это же рекомендации для разработчиков фреймворков. Очевидно, что разработчики фреймворко будут выпускать новые версии своих фреймворков. Также понятно, что одной из важнейши задач для них - сохранение обратной совместимости, по мере возможностей. А значит, чт клиентов, что используют их код, следует максимально ограничить. То есть, это мы (н или только я такой рукожоп) привыкли писать классы наследуемыми - но если мы пишем фреймворк то нужна достаточно веская причина сделать класс наследуемым. Также нужна веская причина сделать метод виртуальным. Но вот ты сделал публичный метод виртуальным, и теперь клиенты могут наследоваться, перегрузить метод и запускать сами написанный ими же код, используя наш фреймвок - и мы уже не можем это контролировать. Я к тому, что делая публичный метод виртуальным, мы даем право клиенту решать, что будет делать наш АПИ и мы уже никак не можем ничего изменить, не сломав обратную совместимость. Однако, сделав защищенный метод виртуальным, мы не гарантируем клиенту, что это метод не перестанет использоваться в будущем, если наш публичный АПИ будет изменен. Таким образом клиенты, что перегрузили защищенный метод, сохраняют обратную совместимость, даже если логика публичных методов была изменена. Походу надо добавить пример, хотя я и не мастер примеров :) Допустим есть следующие классы: public class CsvWriter1 { protected void WriteHeader(TextWriter stream); protected void WriteBody(IEnumerable obj, TextWriter stream); protected virtual void WriteInternal(IEnumerable obj, TextWriter stream) { WriteHeader(stream); WriteBody(obj, stream); } public void Write(IEnumerable obj, TextWriter stream) { WriteInternal(obj, stream); } } public class CsvWriter2 { protected void WriteHeader(TextWriter stream); protected void WriteBody(IEnumerable obj, TextWriter stream); public virtual void Write(IEnumerable obj, TextWriter stream) { WriteHeader(stream); WriteBody(obj, stream); } } Теперь представим, что в следующей версии нашего чудесного фреймворка нам надо писат объекты в CSV без заголовка. То есть, заголовок больше не нужен. И переопределять это больше нельзя. Что делать? Для класса CsvWriter1 все просто public class CsvWriter1 { protected void WriteHeader(TextWriter stream); protected void WriteBody(IEnumerable obj, TextWriter stream); protected virtual void WriteInternal(IEnumerable obj, TextWriter stream) { WriteHeader(stream); WriteBody(obj, stream); } protected void WriteInternalNew(IEnumerable obj, TextWriter stream) { WriteBody(obj, stream); } public void Write(IEnumerable obj, TextWriter stream) { WriteInternalNew(obj, stream); } } В случае с классом CsvWriter2 мы в луже. Так как указанное изменение для него будет обрано несовместимым, и конечно сломает логику клиентских классов.

Ответ 2



Впервые идею невиртуальных интерфейсов я встретил у Герба Саттера еще в начале нулевых и тогда мне это казалось отличной идеей (что не удивительно, я писал на С++ и там эта практика была весьма популярна в принципе, да еще и описана светилами индустрии). И вот его же (Саттера) статья - Virtuality - с объяснением этой темы (найдено в том самом посте, на который я ссылаюсь в предыдущем абзаце): Traditionally, many programmers were used to writing base classes using publi 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 doin two jobs: It's specifying interface because it's public and therefore directly par of the interface Widget presents to the rest of the world; and it's specifying implementatio 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 назад, я старался пользоваться этой идиомой, просто по привычке. Но потом, начал на нее забивать. Теперь, можно поанализировать этот совет. Советом практически никто не пользуется в мире .NET-а. Даже .NET Framework заби на это с большой колокольни. Совет разумен, если рассматривать его, как частный случай Шаблонного метода. Есл в базовом классе определено какое-то поведение: валидация аргументов, базовая фунциональность, хоть что-то. Тогда разделение невиртуального метода открытого интерфейса и защищенного метода, дополнияющего функционал, имеет смысл. Выводы: со всем уважением к Framework Design Guidelines и к Саттеру лично, но н этот совет можно и нужно забить. Плодить методы ради того, чтобы плодить методы - эт никуда не годное время препровождение. Я этим советом очень и очень давно не пользуюсь и не помню ни одной проблемы из-за этого. И да, этому же совету не следуют авторы фреймворка и тоже особых проблем не имеют (из-за этого). Ув. @Denis Gladkiy задал такой вопрос: А какие-нибудь аргументы против NVI по делу-то будут? То на него стоит ответить более явны образом: NVI - это дополнительная работа, а значит, что не нужны аргументы "против". Нужны аргументы "за", ибо YAGNI и преждевременное обобщение. Я не видел в индустрии акцент на этот паттерн нигде, кроме С++. Мое предположение, что он появился в .NET, поскольку его авторы пришли из мира С++. В .NET Framework много примеров нарушеня этого принципа, начиная от System.Object, заканчивая классом Stream Этому правилу уже очень много лет, но новый АПИ, который появляется или расширяетс в .NET Framework-е (тот же Stream.FlushAsync) все еще ему не следует. Если бы этот принцип был бы полезен, а не следование ему было бы плохим, но новый АПИ, который не связан обратной совместимостью уже бы ему следовал. Основываясь на собственном опыте или опыте коллег, я не вижу смысла в следовани этому паттерну, если только в невиртуальном методе есть какое-то поведение. Я понимаю, что мой опыт и опыт моих коллег - это субъективное мнение, но мне сложно представить ситуацию, когда нарушение этого принципа на самом деле приведет к серьезной проблеме. Я задал этот же вопрос автору Framework Design Guidelines, Кжиштофу Квалине - "Что он думает по поводу этой рекомендации сегодня?". Вот его ответ. If I was writing the guidance today, I would make it a “consider” guideline. Whe 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.

Ответ 3



Публичные методы суть контракт класса. Если публичный метод объявлен как виртуальный это означает, что подкласс легко может переопределить его таким образом, что контракт будет нарушаться. В случае с защищенными методами сломать контракт не так легко (а иногда невозможно) поскольку основная логика жестко зашита в невиртуальном публичном методе, в котором есть точки расширения -- те самые защищенные виртуальные методы. Иногда такой прием можно использовать и для защиты от забывчивых программистов, чтобы, переопределяя метод, они "не забыли" вызвать базовую логику. Что-то типа такого. Поэтому да, "do prefer". Но именно prefer.

Ответ 4



С методом Dispose никаких проблем нет: его логика очень проста, а его контракт раз и навсегда задан Microsoft, и не изменится в будущем. Но обычно в базовом классе есть какой-то более сложный алгоритм, точка расширения которого находится посередине, а не с краю. Потому и нужен отдельный метод: interface IValidator { ValidationResult Validate(object obj); } class TypedValidator : 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); } }

Ответ 5



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

Ответ 6



Плюс пять копеек за NVI: NVI и C# Проблемы в неследовании этому не показать на коде, на простом и понятном примере. Проблемы в сложностях развития и поддерживания кода. Как мне кажется, это сродни пониманию в необходимости делать бэкапы Люди делятся на два типа - тех, кто уже делает бэкапы и тех, что ещё нет. (Бэкапы по привычке уже делают многие, но тестируют свои бэкапы ещё не все, например) и многим другим подобным вещам.

Комментариев нет:

Отправить комментарий