Pull to refresh

Comments 24

В 95% случаев спасает NotifyPropertyChangeWeaver, в остальных 5% (зависимости от цепочек свойств) можно объявить внутреннее свойство, которое обновляется вручную по внешним событиям и по которому плагин будет определять изменения в вычисляемых свойствах. Учитывая, что этот плагин сильно упрощает жизнь и крайне прост в использовании (он автоматически распознает вычисляемые свойства, если они состоят только из геттера — атрибуты не нужны), не понятно, зачем еще один велосипед.
Собственно, NotifyPropertyChangeWeaver описан в «Конкурентах», его возможности там рассмотрены. На мой взгляд, возможностей у него меньше, а трекер не менее прост в использовании. По поводу соотношения 95% и 5% — это, все же, определяется «жирностью» ViewModel и, в зависимости, от опыта, может варьироваться. Мне вот, например, в одно время, часто требовались вычислимые свойства «поверх» ObservableCollection, NotifyPropertyChangeWeaver мало мог в этом помочь.
Статью не прочитал еще до конца, но уже есть, возможно, несущественное замечание.
Мой комментарий уже не первый, так что могу себе позволить побрюзжать.

Неудачно Вы выбрали шорткат для ViewModel: «модель». Такой выбор у знающих паттерн MVVM постоянно будет вызывать путаницу.
Это в свою очередь нарушает SRP на микроуровне, так как изначально не зависящие ни от чего (и простые в реализации) свойства теперь вынуждены иметь знания о существовании других свойств и деталей их реализации для того, чтобы иметь возможность в нужные моменты правильно эти свойства обновлять.

Если перейти по ссылке, то в самом начале написано, что SRP есть понятие применяемое к классу. Конкретно в данном случае «якобы SRP» нарушен конкретно указанной реализацией.
Достаточно, например, сделать байндинг на стоимость read-only и при изменении остальных двух свойств делать вычисление и выставку стоимости в ViewModel, и сразу противоречие иссякнет.
С технической точки зрения не все хорошо, потому что могут существовать достаточно сложные связи между свойствами, поддерживать вручную которые достаточно трудоемко (и багоемко). Примерами таких связей являются:

зависимости от свойств, которые сами являются зависимыми;
зависимости от свойств вложенных объектов (цепочки свойств), как в случае DiscountSum = Order.Sum * Order.Discount.Percent / 100;
зависимости от свойств элементов коллекции (TotalQuantity = Orders.Sum(o => o.Quantity)).

А вот это уже понятный довод. Но как и написали в комментарии выше, в сети можно найти реализации аттрибутов для свойств или других способов, реализующих зависимости между свойствами.
//Определяем статическую "карту зависимостей", которая будет хранить зависимости для класса
        private static readonly IDependenciesMap<Order> _dependenciesMap = new DependenciesMap<Order>();

        static Order()
        {            
           //Определяем и добавляем в карту зависимости
            _dependenciesMap.AddDependency(o => o.Cost, o => o.Price * o.Quantity, o => o.Price, o => o.Quantity)
        }

        private IDisposable _tracker;

        public Order()
        {
            //Начинаем отслеживать зависимости для текущего экземпляра модели
            _dependenciesMap.StartTracking(this);
        }


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


Я бы в Вашей статье разместил «АХТУНГ» (большими красными буквами), что ОБЯЗАТЕЛЬНО следует вычищать статически определенный _dependenciesMap, а не то возможны утечки памяти.

И тут сразу вопрос, а как, когда и кто, по-вашему, должен диспозить _dependenciesMap?
Я думаю StartTracking не хранит ссылку на переданный инстанс, поэтому утечки памяти не будет.
_dependenciesMap это статическое поле класса, диспозить его не нужно. Вычищать обязательно тоже ничего не нужно. В _dependenciesMap задана карта зависимостей для вычислимых свойств. Диспозить может потребоваться трекер — объект, которые следит за изменениями цепочек зависимостей в конкретном экземпляре класса. Да и то требуется это только тогда, когда вы хотите в какой-то момент жизненного цикла остановить трекинг зависимости. Если не хотите (это обычно, by default), то и не нужно. Объект принадлежит конкретному инстансу, соотв-но, соберется вместе с ним. Естественно, ссылок на него внутри _dependenciesMap нет.
Да, код я слабовато изучил. Заглянул в репозиторий и поглядел на реализацию.

1. В приведенном коде DependenciesMap создается как статическое поле и висит в памяти пока не завершит работу приложение.
2. При каждом вызове StartTracking создается новый инстанс DependenciesTracker, который в приватное поле сохраняет ссылку на DependenciesMap и на текущий инстанс ViewModel.
3. DependenciesMap ссылку на трекер и на инстанс VM не хранит. Вместо этого трекер хранит ссылку на VM и имеет подписку на ее события. Также трекер пользуется ссылкой на статически определенную DependenciesMap.
4. Таким образом, трекер не собирается GC только благодаря подпискам на события VM. Больше на него никто не ссылается в указанном примере. Если собирется инстанс VM, то и трекер уйдет вслед за VM.

Правильно понял? Может стоит в статье описать принцип работы, а то по примеру складывается впечатление, что DependenciesMap может сохранить у себя ссылку на инстанс VM?
Да, вы все верно написали. Наверное, вы правы по поводу необходимых уточнений в статье.
По поводу п.4 в разных примерах у меня по разному (и это конечно, огрехи при написании статьи) — где-то ссылка на трекер есть, где-то ее нет внутри класса. В первом случае, от подписок ничего не зависит, во втором — зависит. Замечу, что второй случай тоже безопасен. Если трекер не смог создать ни одной подписки на путь (например, когда в пути только readonly-поля, это тоже поддерживается), значит, и отслеживать никаких изменений ему не удастся. Значит, можно его собирать :-)
Зачёт! И за сравнение с конкурентами зачёт.

Но есть нюансы…

зависимости от свойств, которые сами являются зависимыми;

Если немного приукрасить реализацию INPC и учесть, что реализация интерфейса обычно находится в базовом классе (а при использовании решарпера можно и атрибутами приукрасить), то код получается более-менее чистым:

public class Order : ModelBase
{
    private decimal _price;
    private int _quantity;

    public decimal Price
    {
        get { return _price; }
        set { Set(ref _price, value, "Price", "Cost"); }
    }

    public int Quantity
    {
        get { return _quantity; }
        set { Set(ref _quantity, value, "Quantity", "Cost"); }
    }

    public int Cost
    {
        get { return _price * _quantity; }
    }

    // ...
}

То есть для таких простых случаев большого смысла шаманить нет. Хотя, конечно, списки зависимых свойств иногда разрастаются, что не очень приятно. Например, при использовании Caliburn.Micro часто возникает куча свойств с префиксом «Can»…

С другой стороны, реализация с помощью трекера выглядит не такой уж простой, и при этом сказывается на архитектуре: read-only свойство использовать нельзя, добавляется поле типа IDisposable, код из свойства выкидывается в статический конструктор… Можно сделать как-нибудь почище?

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

зависимости от свойств вложенных объектов (цепочки свойств)

Часто бывает, что такие свойства только для чтения, поэтому подписок не слишком много.

зависимости от свойств элементов коллекции

Если коллекция одна, то можно обойтись какой-нибудь коллекцией «NotifyingCollection where T: INPC», где у коллекции бросается какое-нибудь событие при изменении любого элемента. Тогда почти всё шаманство с подписками остаётся внутри коллекции.

Если коллекций несколько, то что-то не так с архитектурой. Длинных цепочек надо избегать. Закон Деметры и всё такое.
read-only свойство использовать нельзя

read-only нельзя, а приватный сеттер можно. Это, конечно, не read-only, но наличие а) private setter б) возможности того, что «property changed» диссонанса не вызывает, а скорее даже наоборот — кажется вполне логичным. Проперти должны вести себя как поля? ОК, если проперти меняется, значит, его кто-то поменял, значит, должен быть сеттер. Но это так, пространные размышления. А на практике, согласитесь, не ограчение это вовсе :-) (уж в случае вью-моделей точно).
добавляется поле типа IDisposable

Ну уж :-) В жирной-то вью-модели добавление одного свойства — это усложнение? Особенно в канонической реализации MVVM по статьям MS, где view model — это вообще God Object. Но вообще-то можно это поле и не добавлять — хранить его нужно только в случае, если хочется в какой-то из моментов жизненного цикла view model трекинг остановить. Если не нужно — то и поля не нужно.
Например, хотелось бы, чтобы код можно было оставить в свойстве, а трекер только кидал уведомления об изменении.

Я бы не сказал, что подход совсем плохой. Думаю, он вполне заслуживает права на жизнь. Более того, некоторые конкуренты так и делают. Просто очевидных плюсов, достаточных для того, чтобы все переделать нет, а вот некоторые проблемы видны сразу. А именно, если есть сеттер, куда значение кладется, то совершенно забесплатно, можно получить следующие вещи:
1. Кеширование вычисленного значения. TotalCost = Orders.Sum(o => o.Price * o.Quantity) вообще-то имеет O(n) сложность вычисления.
2. Рейз события только когда значение поменялось, благодаря стандартному if (_property != value). А можно и всегда рейзить — если эту проверку не написать. В любом случае — слово за тем, кто написал сеттер, а не за трекером. Что, конечно, хорошо, так как без навязывания. Иногда проверка на новое значение не нужна, а иногда проверка очень даже нужна — когда зависимости настолько нетривиальны, что появляются циклы, когда A --> B, а B --> A через какие-нибудь сложные цепочки. И простой способ сделать так, чтобы «колебание затухло» — это не рейзить новых событий, если ничего не поменялось.

Если коллекция одна, то можно обойтись какой-нибудь коллекцией «NotifyingCollection where T: INPC»

Разве эту коллекцию не нужно реализовать также, как трекер? Чем подход проще, когда уже есть трекер?

где у коллекции бросается какое-нибудь событие при изменении любого элемента.

А дальше что с ним делать? Событие нестандартное (надеюсь, вы не собираетесь бросать какой-нибудь CollectionChanged с Reset'ом), биндинг его не поймет. Надо, значит, внутри вью-модели подписываться на него и обновлять вычислимое свойство. А если инстанс коллекции поменяли? :-) :-) Надо переподписаться на событие :-) :-) Ну это как-то тот же трекер и получается :-)
ОК, если проперти меняется, значит, его кто-то поменял, значит, должен быть сеттер.

То есть ты отказываешь вычислимым read-only свойствам в праве на существование? :)

Но вообще-то можно это поле и не добавлять — хранить его нужно только в случае, если хочется в какой-то из моментов жизненного цикла view model трекинг остановить.

А вот это, кстати, можно упомянуть. VM не так уж часто реализуют IDisposable, поэтому отписку девать некуда. А без поля и код будет проще.

Кеширование вычисленного значения. TotalCost = Orders.Sum(o => o.Price * o.Quantity) вообще-то имеет O(n) сложность вычисления.

А вот тут есть нюансы.

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

Во-вторых, если подобные вычисления ударяют по производительности (например, в списке миллион элементов), то вычисление функций Sum, Count и прочих прекрасно оптимизируется: при изменении одного элемента достаточно посчитать дельту. Здесь от «кэширования» польза была бы, но трекер такое не поддерживает.

Рейз события только когда значение поменялось

Здесь опять компромисс, и трекер реализует только один вариант. Я хочу иметь возможность выбирать, что для меня дороже по ресурсам: пересчитывать каждое свойство, даже если оно не нужно, или пересчитывать нужное свойство, даже если оно не поменялось. В идеале, конечно, хотелось бы избавиться от обеих проблем, а не только от одной.

Иногда проверка на новое значение не нужна, а иногда проверка очень даже нужна — когда зависимости настолько нетривиальны, что появляются циклы, когда A --> B, а B --> A через какие-нибудь сложные цепочки.

Подобные цепочки — это уже признак, что что-то не так. В идеале их не должно быть, и избавляться от них желательно не проверкой новых значений с прежними, что по сути костыль. Никто ведь даже не гарантирует, что цикл зависимостей приведёт к «сходящейся» паре значений.

Кроме того, все эти проверки избыточны, если все вычислимые через трекер свойства будут read-only и вычисляться геттером. Скажем, если изменилось свойство A, то трекер может запомнить, что значения свойств B и C устарели, но не перевычислять их. Тогда, если гуй заинтересуется изменением, он обратится к геттеру, который либо возьмёт из трекера актуальное значение, либо перезапишет. Что-нибудь в духе:

public string FullName
{
    get { _tracker.Get(() => FirstName + " " + LastName); }
}

где вторым скрытым аргументом Get будет имя свойства через CallerMemberName. И всё, мы имеем и кэширование, и отсутствие перевычислений ненужных свойств, и основную логику свойства прямо в свойстве, и отсутствие лишних полей. Зависимости правда останутся в статическом конструкторе, и в мусоре на одно замыкание больше, ну да и шут с ними.

Я ничего не упустил?

Разве эту коллекцию не нужно реализовать также, как трекер? Чем подход проще, когда уже есть трекер?

Эта коллекция простая как топор, в отличие от трекера. :) А так разницы мало.

А если инстанс коллекции поменяли?

Нормальные люди так не делают.
Один из конкурентов — ReactiveUI

public class ViewModel : ReactiveObject
{
	public ViewModel()
	{
		_fullName = this.WhenAny(x => x.FirstName, y => y.LastName, 
			(firstName, lastName) => firstName + " " + lastName)
			.ToProperty(this, x => x.FullName);
	}
	private string _firstName;
  	public string FirstName 
  	{
  		get { return _firstName; }
  		set { this.RaiseAndSetIfChanged(ref _firstName, value); }
  	}

  	private string _lastName;
	public string LastName 
  	{
  		get { return _lastName; }
  		set { this.RaiseAndSetIfChanged(ref _lastName, value); }
  	}

  	private ObservableAsPropertyHelper<string> _fullName;
  	public string FullName 
  	{
  		get { return _fullName.Value; }
  	}
}
И обладает всем набором недостатков (глубоко не копал, так что поправьте, если ошибаюсь):
Не поддерживает цепочек свойств и зависимостей от элементов коллекций.
: ReactiveObject

Требует наследования от специального класса.

ObservableAsPropertyHelper _fullName

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

        public void RegisterPropertiesDependencies(string propertyName, List<string> dependenciesProperties)
        {
            foreach (var dependencyProperty in dependenciesProperties)
            {
                this.PropertyChanged += (sender, args) =>
                {
                    if (args.PropertyName == dependencyProperty) RaisePropertyChanged(propertyName);
                };  
            }
        }

        ...
        RegisterPropertiesDependencies("FullName", new List<string> { "FirstName", "LastNameName"});


Для коллекций вложенных объектов пробегать в цикле по их PropertyChanged (для этого использовать какой-нибудь ObservableCollection, чтобы отследить добавление нового объекта и подписаться на его PropertyChanged).
Сделать так, как вы пишете, разумеется, можно. Но на мой взгляд это достаточно трудоемко. Было бы интересно увидеть актуальный код от вас, который поддерживает, допустим, цепочку с коллекцией типа TotalCost = o.OrderProperties.Orders.Sum(o => o.Price * o.Quantity), а также оценку сколько по времени у вас заняла реализация всего этого.
Ответил в виде статьи — вдруг кому-то ещё будет интересно посмотреть. Код писал примерно час-полтора.
Имхо, вероятность изменить алгоритм вычисления свойства и забыть вызвать PropertyChanged в нужном месте примерно равна вероятности забыть указать зависимость в менеджере зависимостей. Сложность обеих операций тоже примерно одинакова. Knockout Style Observables решает проблему необходимости указывания зависимостей (при условии, что в expression не встречаются вызовы кастомных методов). Не знаю, может он и не умеет обрабатывать вложенные коллекции, но его этому можно научить — достаточно отслеживать в дереве выражений вызов методов-расширений над IEnumerable.
А какую проблему решает ваш проект? Незначительно увеличивает cohesion? Вы думаете, что незначительное увеличение cohesion окупает усложнение кода путём добавления нового проекта? Вижу, что проделано много работы, но пока что не понимаю, для чего.
Cohesion тут, конечно, почти не при чем. Как и написано в статье решается проблема сложности ручной поддержки нетривиальных зависимостей. KSO решает только одну проблему — простые одноуровневые зависимости. Если у вас только такие зависимости, то без особых проблем можно использовать как KSO, так и любой из конкурентов. Как и в предыдущем комментарии, хотелось бы увидеть код, который «научил» бы простым образом KSO работать с коллекциями. В частности, интересно сколько времени займет аккуратная реализация обработки каждого из типов событий INotifyCollectionChanged.
Вы думаете, что незначительное увеличение cohesion окупает усложнение кода путём добавления нового проекта?

Использование KSO — это тоже добавление нового проекта :-)
Sign up to leave a comment.

Articles