Pull to refresh

Comments 65

Кстати, о «подстановочном критерии Лискова» упоминал Джеф Элджер, в книге C++. Там, где говорил о нормальном наследовании и гомоморфных иерархиях. Так вот, относительно контракта Элджер упомянул, что если функция базового класса является чисто виртуальной, то все ее вторичные эффекты (которых на самом деле нет) сохраняются по определению. А так как речь идет об интерфейсах, раз они не задают поведения, то и никакого контракта по сути нет. В этом действительно есть здравый смысл.
Барбара Лисков — женщина, при этом очень известная. Извините за занудство.
Прощаю, но это цитата из книги, там именно так и было написано, поставлю минус переводчику.
Напишите в начале статьи о том, что внизу есть определение принципа замещения Лисков.
Дык, так же стимул есть;)
UFO landed and left these words here
Обоснуйте плз, поскольку если класс DoubleList нарушает LSP, то так же делает и HashSet, и SortedList, а раз LSP нарушается классами из BCL, то чем DoubleList хуже? ;)
А если серьезно, то все таки почитайте «большую запутанную статью», выводы в ней никуда ничем не притянуты.
UFO landed and left these words here
На всякий случай напомню, что HashSet не обязательно добавляет элемент. Нарушает ли он тем самым LSP?
UFO landed and left these words here
Вот держу перед глазами книгу Роберта Мартина «Принципы, паттерны и методики гибкой разработки на языке C#», страница 173:
«Принцип подстановки Лисков (Liskov Substitution Principle).
Должна быть возможность вместо базового типа подставить любой его подтип.

Ваше же определение взято из его оригинальной статьи, опубликованной в C++ Report и привязано к языку С++, приведенной мною определение адаптировано автором к языку C#.
Не знаю про HashSet (это из BCL?), но в популярных (по меркам SO и GH) языках, в которых есть формальные интерфейсы, они лишь список сигнатур. Соблюдение LSP они не гарантируют и даже не подразумевают формально. Всё, что они гарантируют — лишь то, что метод с такой сигнатурой будет реализован и его можно будет вызвать. Что эту реализацию можно будет использовать в клиентах прозрачно хоть в каком-нибудь предположении о поведении этого метода, они никакой гарантии или даже информации не дают, только синтаксическое соответствие, никакой семантики. Соблюдение LSP заключается не в реализации интерфейса, а в соблюдении его семантики (как правило неформальной). Отчасти это подтверждается существованием языков, где интерфейсов и/или абстрактных классов нет (или не было) как сущности языка, но, тем не менее, LSP в них можно не нарушать без модификаторов типа final.

Интерфейсы и/или абстрактные классы, как и весь механизм наследования и реализации в популярных языках — лишь инструменты, помогающие не нарушать LSP, но не запрещающие такое нарушение. Выстрелить себе в ногу (нарушить LSP) они позволят без нотайсов и варнингов, даже в дев-окружении (если ассерты не вставлять). Синтаксический сахар для выявления ошибок на этапе статического анализа и, иногда, уменьшения накладных расходов в рантайме в конкретной реализации рантайма.
Да, HashSet — это часть BCL.

Дык весь смысл этой заметки как раз и состоит в ответе на вопрос: а вправе ли мы ожидать от классов реализующих интерфейс ICollection of T определенной семантики или нет. И под конкретной семантикой в данном случае подразумевается: должен ли метод Add обязательно добавлять *еще один элемент* или нет и должен ли быть это только один элемент.

Т.е. по сути, все это попытка найти постусловие метода Add и доказать, что постусловие вида: NewCount = OldCount + 1 не может применяться для этого метода.
В рамках языков таких (C++, C#, Java, PHP — из тех что знаю) тогда вообще об интерфейсах в контексте LSP говорить нельзя. Берём класс, реализующий это интерфейс, наследуемся от него и обсуждаем нарушает ли наследник или нет — интерфейс ненужная деталь в этом случае. А два класса, реализующие этот интерфейс отношениями наследования не связаны и LSP к ним не применим в принципе.

Об этом статья? :)
В с++ же нет интерфейсов, сначала написал без него, потом подумал что чего-то не хватает и дописал зачем-то :) Будем считать, что в C++ это относится к абстрактным классам — их экземпляр создать нельзя, а значит о нарушении LSP тоже можно не беспокоиться.
А не нарушается ли принцип вообще при любом переопределении виртуального метода (не чистого)? Ведь мы его для того и переопределяем, чтобы изменить поведение (с точки зрения базового класса). А описали виртуальным, чтобы можно было переопределить.
Если с точки зрения клиента оно не изменилось, то нет. Грубо говоря клиенту всё равно куда вы сохраняете объект в методе save если он может его получить методом load. Если не всё равно и он минуя эти методы лезет в базу, а там нет ничего, потому что всё в мемкеше лежит — то нарушается. Но при этом нарушение LSP будет не самое страшное наверное :) А уж если по методу save вообще ничего не происходит, а метод load выводит окно на экран…
Поведение изменилось — строки стали сортироваться по-другому (в порядке отличном от дефолтного), на экран стали выводится не точки, а смайлики…
Суть, по-моему, в том, что нужно ли нам изменять остальной код, если где-то вместо родительского класса мы поставим его наследника. Нужно — нарушается, нет — не нарушается. Если мы его подставили, чтобы порядок сортировки изменился — всё ок, если приходится их ещё раз сортировать — нарушение.
Но в переопределенном методе потомка можно вызвать родительский метод, тем самым сохранить контракт.
Мне видится что нарушение контракта будет тогда, когда, например, базовый класс на каждый вызов метода Add вызывает метод слушателя OnAdd, это то чего мы ожидаем, т.е. его побочный эффект. Если в потомке не будет вызываться метод OnAdd слушателя — контракт нарушен.
Все зависит от контекста — от разрабатываемого приложения, от сложившихся соглашений в команде, от мнения тим-лида =)
В целом надо смотреть, как большинство коллег, которые будут потом читать ваш код, понимают назначение метода Add.
Формальные контракты слишком сложны.
Зря «мнение» (не важно чьё) поставили на последнее место.
А почему у вас слово «мнение» в кавычках?
Это ведь хорошо, когда есть авторитет, который быстро поставит точку в спорах, сколько элементов должен добавлять метод Add.
Потому что указатель/ссылка на элемент списка, без семантики :)

Насчёт авторитета я поспорил бы, но это будет большим оффтопиком. Лишь намекну — фамилия «Попов» в контексте веб-разработки вам ни о чём не говорит?
Есть такой авторитет, по советам которого ежедневно генерируется тонны говнокода.
Я не знаю кто такой Попов и чем он занимается.
Я говорил о локальных авторитетах — о лидерах команд.
В команде должен быть общий словарь терминов. Он всегда появляется.
Кстати, если включить стандартные дотнетные CodeContracts, то они уже написаны в фреймворке в том числе и для стандартных интерфейсов типа ICollection и они в постусловии проверяют, что после вызова Add() Count увеличится на 1.
Столкнулись с этим, когда пробовали включить контракты на своем проекте и сломались тесты на «ленивые» коллекции, которые после Add в реальную коллекцию ничего не добавляли, а делали это при перечислении или вызове Count. В итоге, из-за вызова Count контрактами, вся ленивость потерялась.
Сейчас постусловие метода Add ICollection ослабили, теперь там постусловие такое:
{code}
Contract.Ensures(Count >= Contract.OldValue);
{code}

Но да, в результате Count будет вызвано при включенных постусловиях.
Как уже сказано в одном из комментариев — да, нарушает. И HashSet тоже.
В принципе, в этом всём проблемы никакой и нет.

Но меня терзает вопрос: а в чём смысл статьи то? :)) Просто очень странный топик и выбранный пример.
Смысл статьи в том, что нельзя говорить о нарушении LSP до тех пор, пока не определены контракты для интерфейса/базового класса.
Ну при таком раскладе смысл повествования начинает проясняться, спасибо
Не нарушает он ничего. НЕ пишите пока не прочитали.

Есть интерфейс, а есть неформальный контракт. И icollection не говорит ничего о том чему должен быть равен count после Add. И никаких предположений в коде делать не стоит.

То что у кого то там вроде бындю нарушается lsp говорит лишь о том что он писал свой код делая предположения о реализации, что делать очевидно не стоит. Ну либо выдумал он просто выдумал некий контракт про +1 к count, которого очевидно разработчики BCL не описывали.
Пока не прочитал что? Статью прочитал, иначе как я буду её комментировать :)

Раз так, давайте вместо необоснованных нападок ещё раз внимательно прочтём написанное: один член команды решил реализовать свою версию IListOfT. Окей. Что это значит? Есть некий метод, берущий на вход объект (ссылку на объект) типа IListOfT и проводящий с ним некие операции. Что такое IList? Список. Что я ожидаю от его метода Add? Добавление в него 1го элемента. Читаем статью дальше: «если для каждого объекта o1 типа S существует объект o2 типа T такой, что для всех программ P, определенных в контексте T, поведение P не изменяется при замене o1 на o2, то S является базовым типом для T». Если мы подменим IList на DoubleList, поведение нашей программы изменится. Опа! Контрпример для определения! Делаем логичный вывод — принцип нарушен. Разве нет?

И не нужно здесь тыкать носом в ICollection. В данном случае это самый настоящий софизм. По условию задачи мы работаем со списком. Если же коллекция будет менее строгого типа, такого как ICollection — здесь реально уже совсем другой разговор. Мы не можем знать её природу, делать предположения о поведении и тем более опираться на то, что после добавления элемента, кол-во увеличится на один. Потому что множество — это тоже коллекция. Но дубли оно не позволяет.
Почему использование коллекции вместо списка — это софизм? Если вы откроете документацию к интерфейсу IList of T или классу List of T, то увидите, что метод Add берется из интерфейса ICollection of T.

Главный вопрос из определения LSP: что такое «поведение P не изменится»? Каким образом вы определяете, что оно изменилось и почему кто-то рассчитывает на то, что метод Add обязательно должен добавлять элемент?
Про поведение: ну допустим, у программы есть спецификация :))) И «изменится» — это поведение, отличное от данной спецификации. Для примера можно взять хоть те же тесты. Про «кто-то рассчитывает на то, что метод Add» — понимаю, странно. Скорее всего, программист делает что-то не так. Но он же всё-таки имеет на это право? Ему дали список и опираться на стандартное поведение списка — разумно.

Про ICollection: я не спорю что в BCL список является наследником коллекции. Но также стоит понять, что список — это коллекция с дополнительными условиями, более узкое понятие, более строгое. Нельзя просто так скормить программе объект с менее строгим поведением. Найдётся случай, когда всё упадёт. Который как раз и описан в статье :) А вот заменить IList на более специализированный список — можно. Принцип Лисков как раз об этом и говорит. И заменив IList на ICollection вы уже его и нарушили! DoubleList уже перестал играть здесь свою роль.
Ну сразу, любой базовый класс или интерфейс по определению является более *базовым* понятием, а наследник — это более узкое понятие. Так что коллекция — это более базовое понятие, и поэтому замена листа на коллекцию является корректным.
Я могу показаться некорректным, но после такого утверждения могу сказать лишь: «садись, два» и «RTFM».
Сори, не правильно понял, но и Вы, видимо тоже (поскольку мы написали одно и тоже): «список — это коллекция с дополнительными условиями» (вы), т.е. «коллекция — это более базовое понятие» (я).

Но, к сожалению, если посмотреть на реальные постусловия метода Add коллекции и списка, то их постусловия являются одинаковыми (у обоих оно
Contract.Ensures(Count >= Contract.OldValue(Count);


Можете поставить контракты, чтобы в этом убедиться.
Я просто толкую о том, что заменить IList на ICollection без риска сломать программу — нельзя. Не знаю почему вы с этим так упорно спорите. Посмотрите хотя бы MSDN и увидьте, что у списка есть свои личные методы. Программа, использующая их, если ей на вход подсунуть ICollection — банально не скомпилируется.

В обратную же сторону, если программа оперирует ICollection, то подсунув ей IList — поведение не изменится.
Сергей говорит что можно в месте использования заменить IList на ICollection и если лишних методов IList не используется то замена будет корректна.

То что не должны быть использованы методы ILIst — это было слишком очевидно и подразумевалось. Вы просто не поняли
Не хочу продолжать и усуглублять дискуссию, вроде бы пришедшую к консенсусу. Но нельзя в общем случае заменять потомка на его родителя. Никто не будет гарантировать корректность результата.
Я говорю лишь о том, что ребята, которые спорили о DoubleList спорили бы точно также и об DoubleCollection, поскольку с точки зрения именно метода Add семантика у обоих этих интерфейсов одинаковая.

Да, список — это более конкретный тип, но с точки зрения добавления элементов семантика их обоих — одинаковая.

З.Ы. +100 500 на счет того, что родителя нельзя заменять на потомка *в общем случае*.
Более того, вчера уже перед публикацией этой статьи мы обсуждали с коллегой именно мой «грязный хак» с рассмотрением коллекции, вместо списка. Коллега предположил, что список содержит более строгое постусловие вида: count = old cound + 1, а коллекция содержит более слабое постусловие.

После этого, мы открыли сборку с установленными контрактами и убедились, что постусловие обоих этих интерфейсов для метода Add одинаковы (хотя постусловие наследника спокойно может быть сильнее).

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

Любой инженер — это бывший школьник. Но далеко не любой бывший школьник — это инженер. Инженер более широкое понятие (в смысле дает более широкие возможности), чем бывший школьник.
1. IListofT не определяет метод Add, а наследует его от ICollectionofT. Count кстати тоже.

2. Count возвращает кол-во элементов в коллекции (это читайте на msdn). Никакой связи между Add и Count там не описано.

DoubleList — а если корректнее — DoubleCollection — не нарушит LSP если Add Будет увеличивать Count на 2.
А я о чём? «а если корректнее — DoubleCollection». Такой класс — не нарушит.
Мужчина, кстати, свою мочеполовую систему наследует от класса Человек. Но при этом, для облегчения после пары литров пива, ему не обязательно нужно садиться и спускать штаны — хватит и забора да расстёгнутой ширинки. Почему же тогда женщине будет так некомфортно пользоваться писсуаром?
Человек — это абстрактный класс, и я бы сказал что там нету реализации мочеполовой системы.
Но я скорее не понял примера.

Если вернуться к теме:
У автора в листингах DoubleList наследуется от ICollectionofT. Я с вами соглашусь в том что название было выбрано неверно.

Если начать разговор именно об IListofT — нужно рассматривать контракт индексатора и метода IndexOf. Вот тут опять же — я скорее соглашусь с вами, насчёт некорректности добавления второго эл-та. Но повторюсь, не из за Count & Add, а именно из за логики индексатора.
Ну дык весь сыр-бор из-за "… предположим, один из членов команды пытается реализовать интерфейс IListofT в классе DoubleListofT таким образом, чтобы ...". Откуда же читателям знать, что «У автора в листингах DoubleList наследуется от ICollectionofT»? Это ж принципиально!

Про Человека: я проводил аналогию с туалетом и Человеком. Туалет не зря есть М и Ж. И женщинам будет некомфортно пользоваться М. Поэтому в реализации конкретного туалета нельзя опираться на абстрактного Человека.
У вас неправильная аналогия.

Поправлю вашу аналогию.

Представьте, себе туалет (метод AddItem) где можно только руки мыть но не писать, который принимает абстрактного человека (ICollection) — ему можно передать и мужчину (IList) и женщину (IDictionary). (ликбез: лист и дикт — оба наследуются от ICollection)

Теперь представьте 2 вида настоящего туалета с писуарами и кабинками(InsertItem, InsertKeyValue) — ему уже не подойдёт абстрактный человек(ICollection). Один вид будет принимать IList, Второй — IDictionary.

В случае туалета который только с раковинами (без писуаров и кабинок) — вы можете в него передать любую имплементацию (Мужчина 34 лет, с бородой) и он отработает корректно просто потому что навык мытья рук(Add) и кол-во рук (Count) — это часть абстрактного человека.

Кто разрешал исправлять мою аналогию? :)
Возможно, ваша версия аналогии — это реальное положение вещей в коде, который повлёк за собой данную статью.
Моя версия аналогии — это то, что в этой статье отражено. Есть некий абстрактный код, мы не знаем что конкретно он делает с входным параметром. И есть этот параметр — список. В общем случае обобщать параметр с IList до ICollection — нельзя. Ибо мы не знаем, используется ли там метод IndexOf или что-нибудь из этой оперы. Не зна ем. А вы, видимо — знаете. Но в статье это не описали :)
Я понимаю, что сейчас солидно соблюдать разные SOLID правила. Но может объясните, ЗАЧЕМ? соблюдать принцип Барбары? (мне видится, что он навеян устаревающей технологией COM, а вот зачем он вообще — совершенно не ясно). Или конкретнее, как может поток-класс соблюдать нотацию родителя, если собственно потомок делается для того, чтобы переопределить/уточнить поведение родителя.
А как связана бедная Барбара с COM, если принцип замещения появился в 1987, а COM в 1993?
Не прямо конечно. Сравните принцип Барбары с «однажды созданные (опубликованные) интерфейсы COM-компонента не изменяются». Найдите 10 отличий?
Вот оригинальное (на всякий случай) определение LSP:

If for each object o1 of type S there is an object o2 of type T such that for all programs P defined in terms of T, the behavior of P is unchanged when o1 is substituted for o2 then S is a subtype of T

Пока не вижу связи с интерфейсами COM-компонента.
Попробуйте соблюсти принцип «однажды созданные (опубликованные) интерфейсы не изменяются» и нарушить принцип Барбары.
Если уж о что-то соблюдать — то надо избавится от всех предусловий/постусловий. Все предусловия/постусловия должны быть определенны как входные/выходные параметры метода. Если есть какие — то другие предусловия/постусловя, то метод должен проверять свое (объекта) состояние и выдавать ошибку. Так например, HashSet должен выдавать исключение если не добавляет ОДИН элемент. И так оно и есть в C# — Hashtable.Add выдает исключение ArgumentException если элемент с таким ключом уже существует в Hashtable. И никаких спекуляций с совершенно не интересным принципом Барбары. Избавляйтесь от предусловий/постусловий, или контролируйте их исключениями.
И кстати, у вас ошибка. Смотрим сюда и видим, что SortedSet(Of T).Add это реализация ISet(Of T).Add(T). И нет там реализации ICollection.Add.

А разница как раз в том, что постусловия переопределяются через выходной параметр — true if item is added to the set; otherwise, false.

И естественно тут нет никакого нарушения — разные интерфейсы
Явная реализация интерфейса не запрещает использовать класс через интерфейс:

void AddItemToCollection(ICollection c, string s)
{
// здесь мы видим именно метод ICollection.Add, который не возвращает булевого параметра
}

void Foo()
{
var ss = new SortedSet();

// Здесь мы спокойно приводим к интерфейсу и используем SortedSet полиморфно
AddItemToCollection(ss, "hello");
}
Это ясно. Т.е. есть на самом деле вопрос — как правильно закрыть ненужную реализацию интерфейса.
В таком случае, все таки, если ((ICollection)SortedSet).Add() молча не добавляет элемент — он нарушает принцип неизменности интерфейса (или более частный принцип Барбары). Правильно же было бы поступить как в Hashtable кидать ХОТЯ БЫ исключение.

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

И Вы тут нашли хороший пример, когда НУЖНО нарушать принцип неизменности опубликованного интерфейса. И сами разработчики C# его нарушают.
Точнее есть, но закрытая.
Да очень просто.

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

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

Если бы в коде не возникали костыли, то было бы абсолютно не важно нарушается принцип или нет и вообще существует он или нет.
Only those users with full accounts are able to leave comments. Log in, please.