Pull to refresh

Comments 37

Один из вариантов – создать для объекта read only интерфейс, из которого исключить все методы, изменяющие объект.

Т.е. ваш класс мог бы поддержать интерфейс IEnumerable(T) или IReadOnlyCollection(T). Если добавить к ним ещё IReadOnlyList(T) и IReadOnlyDictionary(TKey, TValue), то в большинстве случаев этого будет достаточно чтобы защитить коллекции от изменений.
Да, согласен. Но я здесь рассматриваю более общий случай. То, что в примерах вектор, — это для простоты. Это могла бы быть и матрица, к примеру, для которой придётся создать свой readonly-интерфейс. К тому же, вышеозначенные интерфейсы так же не спасут от даункаста, если класс находится в той же сборке или видим извне.
Дело не в конкретных интерфейсах, а в подходе. Например:

interface IReadOnlyVector
{
	// readonly methods and properties
}

interface IVector : IReadOnlyVector
{
	// any methods and properties
}

class ReadOnlyVector : IVector, IReadOnlyVector
{
	private readonly IVector vector;

	public ReadOnlyVector(IVector vector)
	{
		this.vector = vector;
	}
}

class Vector : IVector, IReadOnlyVector
{
	public IReadOnlyVector AsReadOnly()
	{
		return new ReadOnlyVector(this);
	}
}


Защитит вас от downcast. В С++ const и упомянутые интерфейсы для меня это прежде всего контракт, соглашение если угодно, и если пользователь моей библиотеки играет не по правилам, т.е. проводит явное преобразование, то я необязан гарантировать корректное поведение.
class ReadOnlyVector : IReadOnlyVector

Конечно же.
Это ровно то же самое, что и у меня. Только у меня в ReadOnly сохраняется ссылка на сами данные класса (T[]), а не на сам класс. И вместо моего свойства Reader у Вас AsReadOnly(). И ещё у Вас Vector наследует от IReadOnlyVector, что заставит Вас реализовывать этот интерфейс дважды. И повторюсь, не всё является обычными коллекциями. А гарантировать на 100% из-за Reflection'а никто и не может, просто так надёжнее, потому что обойти это сложнее. Да и код разделён на два класса, что делает его читабельнее.
Это ровно то же самое, что и у меня. Только у меня в ReadOnly сохраняется ссылка на сами данные класса (T[]), а не на сам класс. И вместо моего свойства Reader у Вас AsReadOnly().

Реализации выглядят, похоже, но исключенно дублирование данных и кода реализации. Кроме того, приведено к виду используемом в .Net Framework (пример — List(T).AsReadOnly)
И ещё у Вас Vector наследует от IReadOnlyVector, что заставит Вас реализовывать этот интерфейс дважды.

Что, простите? Не могли бы вы пояснить эту мысль примером с кодом?
И повторюсь, не всё является обычными коллекциями.

Мой предыдущий комментарий не содержал ни слова о коллекциях. Вы можете заменить все вхождения слова vector на user или order и т.д. и получить универсальный паттерн.
Насчёт коллекций извиняюсь, мне померещился IReadOnlyCollection в коде :)

ReadOnlyVector у Вас является оболочкой над Vector, но методы-то в нём Вам всё равно реализовывать. Это и есть то дублирование, которого у меня как раз-таки и нет. Но если так трудно вызывать Reader, можете мысленно и в мой код добавить такое наследование, сути это не меняет.

Кстати говоря, я в статье постоянно упоминал про производительность, а Ваш wrapper — это мягко говоря, не очень эффективное решение.
ReadOnlyVector у Вас является оболочкой над Vector, но методы-то в нём Вам всё равно реализовывать. Это и есть то дублирование, которого у меня как раз-таки и нет.

Вся реализация сводится к делегированию подмножества методов для чтения защищаемому объекту. По сути, это реализация паттерна Адаптер.
Кстати говоря, я в статье постоянно упоминал про производительность, а Ваш wrapper — это мягко говоря, не очень эффективное решение.

Почему?
Почему?

А лишние вызовы по-Вашему бесплатны? Я отказался от скрытия readonly-структуры, чтобы можно было к ней напрямую обращаться, а тут такое…
Нет, не бесплатны. Но они не настолько снижают производительность, чтобы от них отказываться в пользу дублирования данных, что, в свою очередь, удваивает количество потребляемой памяти. Это в противовес, фиксированной стоимости промежуточного вызова.
Дублирования данных не происходит, дублируется только ссылка на данные, что для качественно написанных объектов ни к чему плохому не приводит. И даже наоборот: когда Vector уже не нужен, он спокойно может быть уничтожен, в то время как VectorConst может существовать дальше. У Вас он в такой ситуации не уничтожится, т.к. на него есть ссылка в ReadOnlyVector.

Но они не настолько снижают производительность...

К сожалению, в ряде случаев настолько. И Вы мало того, что дублируете вызовы, Вы эти вызовы осуществляете через интерфейс, что ещё в разы(!) делает их медленнее.

Для задач, совершенно не требовательных к производительности, это приемлемый вариант, не спорю. Однако я всё равно не вижу причин его использовать. Этот вариант может быть вынужденной мерой, когда объекты уже написаны и Вы не имеете доступа к исходникам. Или же просто нецелесообразно тратить время на переписывание объекта. Адаптер — это костыль.
Некорректно выразился под данными, я имел ввиду конечные реализации.

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

Спасибо за дискуссию, я понял вашу точку зрения.
Я не хочу холивара, но вынужден немного не согласиться…

Я предлагаю избавиться от дублирования кода реализации

В данном случае, дублирования нет.

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

Чтение и запись — разные вещи и внешние методы не повторяются. А если будут появляться скрытые (именно скрытые) общие методы, то опять-таки «Адаптер» — не единственный выход. Можно сделать оболочку не над Vector'ом, а над данными. Но и этого в 99 случаев из 100 делать не придётся, т.к. эти потенциальные методы будут readonly и скрывать их, скорее всего, не придётся.

А в поддержке этот подход, боюсь, будет даже проще благодаря дополнительной декомпозиции.

Паттерны GoF — не есть конечная истина и польза, скорее наоборот :)
В данном случае, дублирования нет.

Ещё раз посмотрел на вашу реализацию и понял, что все методы чтения вы предлагаете делать через Reader. Т.е. если мне не нужен read-only доступ, я как пользователь вашего класса вынужден писать:

var v = new Vector<int>(5);

v[0] = 0;
Console.WriteLine(v.Reader[0]);    

Это так? Вы сознательно поделили read и write версии? Обращение через дополнительно свойство в данном случае не overhead и потеря производительности? Удобно ли это в использовании для конечного пользователя?
Да, это так. Сознательно. Если вызывать свойство Reader каждый раз при любой операции получится то же, что и у Вас. А при передаче VectorConst в другой метод, Reader или оператор преобразования вызовется только один раз. Получается, что, как ни крути, это выгоднее «Адаптера».

Если свойство Reader настолько уж раздражает, можно запихать его в Vector. пометив как private, и перенаправлять к нему реализацию IVectorConst (или Вашего IReadOnlyVector). И даже это всё равно будет лучше «Адаптера», т.к. вызовы будут происходить не через интерфейс и останется разделение ответственности.
можно запихать его в Vector.


Хотел сказать: «можно запихать VectorConst в Vector»
Если вызывать свойство Reader каждый раз при любой операции получится то же, что и у Вас. А при передаче VectorConst в другой метод, Reader или оператор преобразования вызовется только один раз. Получается, что, как ни крути, это выгоднее «Адаптера».

С задачей передачи read-only объекта в метод ваш пример справляется на 100%
Если свойство Reader настолько уж раздражает, можно запихать его в Vector. пометив как private, и перенаправлять к нему реализацию IVectorConst (или Вашего IReadOnlyVector). И даже это всё равно будет лучше «Адаптера», т.к. вызовы будут происходить не через интерфейс и останется разделение ответственности.

Меня как пользователя не устраивает, что я должен писать в одно место и читать из другого. Если вы «запихаете VectorConst в Vector» и «пометите его как private» это никак не повлияет, что мне нужно вызывать Reader каждый раз чтобы что-то прочитать.
Извиняюсь, я что-то не то написал… «Запихать VectorConst в Vector» — это чтобы заставить разработчика использовать исключительно интерфейсы (об этом я писал в статье).

А чтобы убрать необходимость в вызовах Reader достаточно вот этого:
struct Vector<T> : IVector<T>, IVectorConst<T>
{
    ...
    public T this[int nIndex]
    {
        get { return _reader[nIndex]; }
        set { _vector[nIndex] = value; }
    }
    ...
}


То же самое, что у Вас, только отсутствие вызовов через интерфейс, быстрый readonly-объект, контравариантность IVector и разделение ответственности.
Т.е. продублировать все read вызовы из VectorConst в Vector? А в сторонние методы передавать через Reader, чтобы избежать downcast?
Я опять поторопился… Вот так:

struct Vector<T> : IVector<T>
{
    ...
    public T this[int nIndex]
    {
        get { return _reader[nIndex]; }
        set { _vector[nIndex] = value; }
    }
    ...
}

Т.е. без наследования от IVectorConst (это помешает передаче быстрого reader'а в методы).
Ок, мы добрались до места, где ваша полная реализация делегирует методы чтения реализации только для чтения, в то время как мой вариант реализации только для чтения делегирует методы чтения полной реализации. Итого, если отбросить интерфейсы, можно утверждать, что ваша read-only версия читает данные быстрее, т.к. нет дополнительного вызова, в то время как моя full версия читает быстрее по тем же причинам. И могу с увереностью сказать, что вы реализовали Декоратор))
И могу с увереностью сказать, что вы реализовали Декоратор

Декоратор служит для динамического подключения поведения, у меня ничего динамического тут нет.

ваша read-only версия читает данные быстрее, т.к. нет дополнительного вызова, в то время как моя full версия читает быстрее по тем же причинам.

Полностью согласен. С той лишь разницей, что мой подход всегда позволяет получить быстрый reader, даже из full-версии (если знать про него). А также, от моей реализации можно быстро перейти к вашей, а вот на обратный переход может уйти немало времени, т.к. чтение и запись перемешаны.

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

Я рассматривал данный подход в контексте аналогии с cast из C++. А cast не предусматривает никаких дополнительных вызовов, это проверка в compile-time и максимум производительности. Поэтому я и предложил такой подход, чтобы не было ощутимых потерь. Кроме того, этот подход можно использовать и в самом C++, когда нужен более надёжный код, но каждая миллисекунда на счету. Кстати, там для получения reader'а вообще можно поставить __inline.
Декоратор служит для динамического подключения поведения, у меня ничего динамического тут нет.

Необязательно. Вы декорировали ваш читающий класс методами записи.
Кстати, там для получения reader'а вообще можно поставить __inline.

MethodImplOptions.AggressiveInlining, если хотите дать совет JIT.
Необязательно. Вы декорировали ваш читающий класс методами записи.

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

MethodImplOptions.AggressiveInlining, если хотите дать совет JIT.

Замечательно, спасибо, буду знать :) Но это только подтверждает преимущество моего подхода. Можете записать его в коллекцию паттернов :)
Дописал статью, добавив туда «Адаптер». Только убрал из вашей реализации наследование Vector и IVector от IReadOnlyVector, т.к. в этом случае становится возможным беспрепятственно передавать Vector и IVector в методы, которые принимают IReadOnlyVector, без вызова AsReadOnly(). А такое я уже описал в пункте 2. В общем, спасибо в любом случае, приятно было подискутировать.
Всегда пожалуйста, только используйте специальный тег для ссылки на хабрапользователя Ithilgwau.
Спасибо, не знал про него, поправил.
Честно говоря, или статья сумбурная, или я совершенно тупой. Постараюсь по пунктам в соотв. с пунктами статьи.

1. По поводу string — именно для таких случаев существует StringBuilder. Также вы всегда можете использовать Char[].

2,3. Я вообще не понимаю, каким боком интерфейсы относятся к константности/изменяемости объектов. Далее, следует различать константность указателей и константность значений, на которые «ведут» указатели. В разделе 2 ваш Vector изменяем — через индексатор можно менять его содержимое.

4. Я понимаю, что вариантность/инвариантность — это важные и далеко не простые темы, но я так и не понял, как тема константности объектов соотносится с темой вариативности. Кроме того, если уже говорить о связи вариативности с константностью, то приведение типа к «сверхтипу/родительскому типу» не изменяет содержимое объекта, т.е. в коде string[] str = new string[1] { "a" }; object[] o = str;Type t = o.GetType(); переменная t будет показывать String[].

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

Возможно, в меня недостаточно компетентности для понимания вашей статьи, тем более, я незнаком с С++, однако с моей точки зрения статья сумбурна, и я не смог уловить её основной смысл. О чём она — о вариативности типов или о константности объектов? Так и как же правильно делать?
никакие методы не должны менять содержимое

Я правильно понимаю, что это остается исключительно на контроле разработчика? И видя только интерфейс какого-либо класса как пользователь, я не смогу узнать, что методы гарантированно не меняют состояние объекта?

В С++, например, уже на этапе компиляции вызов неконстантных методов у константных объектов приведет к ошибке.
Я правильно понимаю, что это остается исключительно на контроле разработчика?

Совершенно верно. Лично я в описании типа явно указываю, что объект неизменяем, однако вы правы — компилятор о его неизменяемости не знает. Именно поэтому модификатор const применим лишь к примитивным типам: они 100%-но неизменяемы, и это компилятор воспринимает как факт, это в него заложено. Но вот DateTime или TimeSpan тоже совершенно неизменяемые значимые типы, но написать const DateTime x = new DateTime(2013, 10, 29); я не могу — компилятор не знает о неизменяемости DateTime, не может её вывести самостоятельно, и я не могу ему помочь. C# не идеален, и это нужно понимать.
const, применительно к переменным внутри метода — это совершенно другая опера. А DateTime компилятор не даёт делать const потому, что что он не является «compile-time constant», для его инициализации нужно вызвать конструктор, а сделать это можно только в runtime.
1. Если нужно в строке просто заменить один символ, то StringBuilder не спасёт Вас от копирования всей строки. А насчёт char[]… Для этого как раз мой подход и может пригодиться.

2,3 В статье везде говорится о константности значений. Да, Vector изменяем. Но не IVectorConst. И если метод принимает именно IVectorConst, то без даункаста он не сможет изменить данные.

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

5. Тут речь о том, чтобы иметь возможность и изменять состояние объекта, и когда надо запрещать изменения чужим методам. Объект, который Ва описали — это неизменяемый объект, наподобие string. Это хорошая практика, но у неё есть и минусы, о которых я упомянул в первом разделе. А делать можно любым из трёх способов, всё зависит от того, насколько производительный и насколько строгий код Вы хотите иметь… 3-ий вариант — это и то, и другое. Но слишком усложнять простой код тоже не стоит, я думаю.
Воспользовался Вашими пожеланиями и убрал из текста статьи всё касаемо вариативности, чтобы не запутывать лишний раз (только в коде оставил). Но кое-что дописал (добавил ещё один подход). Теперь вроде всё в полном объёме и всё по делу :)
Ситуация: Петя вызывает метод класса, разработанного Васей, и передаёт ему как параметр экземпляр класса, разработанного Мишей.

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

К кому идти Пете, к Васе или к Мише? Кто может лучше успокоить Петю, кто обеспечит гарантии?

Ответ 1: Без Миши никак не обойтись. Если только Миша заранее не предусмотрел у своего класса ReadOnly интерфейс. Если ReadOnly интерфейс уже есть, то Вася справиться и сам. Он переделывает свой метод, так, чтобы он принимал на вход именно этот ReadOnly интерфейс. Петя теперь будет спокоен.

Ответ 2: Если Миша не предусмотрел у своего класса ReadOnly интерфейс, и не собирается это делать? То можно обойтись и без него (и без Васи кстати тоже). Передадим в Васин метод копию экземпляра Мишиного класса. Дорого? Да. Но мы в безвыходной ситуации…

Статья по-моему написана для Миши. Как ему сделать по-настоящему надёжный ReadOnly интерфейс, защищающий от downcast'а.

Ответ 3: Возможен ещё один метод успокоения Пети, упрощающий Мише реализацию ReadOnly интерфейса. Надо чтобы Вася ничего не знал от Мишином классе, а знал только об его ReadOnly интерфейсе. Тогда он не сможет (физически) сделать downcast. И Петя будет совершено спокоен. Миша может в этом случае реализовать интерфейс «по-простому». Но тогда придётся разнести ReadOnly интерфейс и реализацию класса по разным сборкам. Пете дать и реализацию и интерфейс, а Васе только интерфейс. Но возможно это уже слишком?..
Здорово Вы всё расписали! Да, статья для Миши конечно… Впрочем, если все три лица находятся в одном, то это всё тоже имеет смысл. Мы и сами довольно часто отстреливаем себе ногу :) А по разным сборкам — наверное действительно уже слишком. Хотя, наверняка и такое практикуется… Сделали бы const уже, сколько проблем бы это устранило…
Sign up to leave a comment.

Articles