Pull to refresh

Comments 50

Имхо, статье не хватает проверки падения производительности и примера этого АОП в том же классе из начала статьи, например.
Добавил в статью пример сгенерированного кода. Как можно увидеть из примера, падение производительности будет зависеть в первую очередь от сложности конструкторов аспектов и количества аспектов на одном классе, так как создание их экземпляров «врезается» в конструктор «цели». Я подумаю над тем, как лучше выразить падение производительности.
Т. е. у Вас по сути получился Fody+PropertyChanged (или «старый» NotifyPropertyWeaver)? В чём отличие Вашей реализации от них?
NotifyPropertyWeaver — узкоспециализированный инструмент, который предназначен для решения одной задачи. Aspect Injector — инструмент для создания и инжекции любых аспектов, не только для реализации INotifyPropertyChanged. И в отличие от Fody в нем не нужно писать или подключать плагины — достаточно у себя в коде объявить класс аспекта и затем привязывать его к любым «целям». Никаких дополнительных действий кроме подключения Aspect Injector не нужно. Плюс ко всему — отладка без проблем заходит в методы созданных аспектов. Пример сгенерированного кода можно увидеть в статье (недавно добавил).
Именно для INotifyProperty использую www.nuget.org/packages/KindOfMagic/. Вроде работает аккуратно, код чистый. Использовать очень удобно. Но глубоко не анализировал, только проверил что он не генерирует лишнего.
Прекрасная штука и автор молодец. Я тоже её использую!
Тот же бесплатный Fody использует Mono.Cecil -> compile-time assembly weaving.

А что под капотом вашей библиотеки?
Хватит изобреать велосипеды.
Использовать обычный INotifyPropertyChanged руки не отпадут, без всяких но.
Зачем LINQ, когда есть foreach и if
Зачем foreach если есть for
Зачем for, если есть while
Использовать обычный JNZ руки не отпадут.
Вы сравниваете совсем разные вещи.
Потрудитесь объяснить разницу?
INPC и есть удобный, быстрый и гибкий иструмент.
Это все равно если бы qw1 и a1exis предлагали вместо foreach использовать какой-то медленный самописный делегат который, о боже, предоставляет текущий индекс.

Решением для конкретной «проблемы» есть добавление INPC Snippet в студию и будет вам счастье.
Внедрение аспекта ещё быстрее и удобнее) Что касается гибкости: если — тьфу-тьфу-тьфу — в коде нотификации обнаружиться какая либо проблема, то её можно будет исправить лишь изменив код аспекта, а не удаляя/пересоздавая массу портянок. Как писали ниже — INPC лишь пример проблемы, которую можно таким образом решить.
В общем случае любой компилируемый язык без макросов имеет максимальный предел абстракции, аспекты как раз позволяют подняться на уровень выше. Аспект INPC это тоже самое, что event, встроенный в язык — события можно реализовать и без него, но с event удобнее)

P.S. во многих функциональных языках есть mapi и это нормально)
Я тоже согласен, что INPC это плохой пример использования AOP. Слишком много проблем может вызвать наивная реализация INPC как в топике: от ложного вызова PropertyChanged до огромных проблем при многопоточном использовании. Более продвинутая реализация же требует аккуратного настраивания и использования, делает код менее читаемым и понятным, чем реализация «в лоб».

Просто напишите сниппет. Вот мой.
до огромных проблем при многопоточном использовании


Каким образом проблемы многопоточного использования решает ваш сниппет? Из других потоков звать нотификацию нельзя, и об этом нужно явно заботиться, т.е. не изменять свойства вьюмодела напрямую из другого потока.
Каким образом проблемы многопоточного использования решает ваш сниппет?
В том-то и дело, что сниппет этим не занимается! Это зовётся инкапсуляцией. Все проблемы класса разруливаются самим классом.

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

Во-первых, что такое «другой поток» в контексте INotifyPropertyChanged? Для DispatcherObject я понимаю, у него есть Dispatcher, а он привязан к какому-то потоку.

Во-вторых, вы этим предложением на все свойства объекта и члены INotifyPropertyChanged наложили невидимый контракт, запрещающий их вызов из определённых потоков, а также заставляющий PropertyChanged тоже вызываться в каком-то «не другом» потоке. Это нарушение сразу многих принципов объектно-ориентированного программирования. Короче говоря, так делать нельзя.

В-третьих, это искусственное ограничение вашего фреймворка, который почему-то берёт на себя проблемы самого объекта (или инфраструктуры проекта-пользователя). Вот, например, мой концепт вызова PropertyChanged в разных потоках.

И это далеко не полный список проблем с многопоточностью.
Во-первых, что такое «другой поток» в контексте INotifyPropertyChanged?

Согласен, что не совсем корректно выразился. Расшифрую. Чаще всего мы работаем не со «сферическим конем в вакууме», а с конкретной ситуацией, когда INotifyPropertyChanged используется UI фреймворком для получения информации об изменениях свойств. UI работает в «главном» потоке приложения, его же называют «UI поток». И вызов обработчиков события PropertyChanged из любого другого потока в данной ситуации не есть правильным.

Во-вторых, вы этим предложением на все свойства объекта и члены INotifyPropertyChanged наложили невидимый контракт, запрещающий их вызов из определённых потоков, а также заставляющий PropertyChanged тоже вызываться в каком-то «не другом» потоке. Это нарушение сразу многих принципов объектно-ориентированного программирования. Короче говоря, так делать нельзя.

Допустим, у меня есть вьюмодел с множеством свойств, пара из которых может устанавливаться как из UI, так и из фонового потока. Причем установка из UI потока делается намного чаще, чем из фонового. Что делать?

В-третьих, это искусственное ограничение вашего фреймворка, который почему-то берёт на себя проблемы самого объекта (или инфраструктуры проекта-пользователя). Вот, например, мой концепт вызова PropertyChanged в разных потоках.

Насчет искусственного ограничения вообще не понял — не вижу проблем написать аспект, который будет диспатчить вызовы. Приведенные выше примеры аспектов — они не встроены во фреймворк, они могут быть написаны и заинжекчены с помощью него, со всеми нюансами, которые вам нужны. И тут еще на всякий случай замечу, что синхронизация доступа бесплатно не дается, поэтому объявление всех свойств модели как «принудительно синхронизированных» может не очень хорошо сказаться на производительности.
Согласен, что не совсем корректно выразился. Расшифрую. Чаще всего мы работаем не со «сферическим конем в вакууме», а с конкретной ситуацией, когда INotifyPropertyChanged используется UI фреймворком для получения информации об изменениях свойств. UI работает в «главном» потоке приложения, его же называют «UI поток».
Нет такого глобального понятия как «UI поток». Возможно, в вашем приложении оно есть, но вы пишите AOP фреймворк. Вероятно, в вашем UI тулките есть такое понятие, но не у пользователей вашего AOP фреймворка.

Вместо этого в операционной системе Windows есть понятие «HWND поток». У каждого HWND он свой. У двух HWND они могут быть одинаковыми, а могут быть и нет. Но UI потока нет.

И вызов обработчиков события PropertyChanged из любого другого потока в данной ситуации не есть правильным.
Попробую теперь объяснить на пальцах. У вас есть экземпляр INotifyPropertyChanged с единственным свойством: у него есть событие PropertyChanged, на которое можно подписаться и от него отписаться, причём в обработчик при его вызове будут переданы аргументы, присваиваемые переменной определённого типа. Это полный и окончательный список того, что гарантирует вам этот интерфейс. Здесь не говорится ни о каких потоках, пользователь интерфейса о них попросто не имеет представления.

Другой пример. Представьте себе вот такой интерфейс:
interface IMyIface {
    string GetName(); // never null
}
Этот интерфейс обладает единственным свойством: наличием метода GetName, возвращающим не-пустое значение типа string (поскольку этот тип нерасширяемый). C# по-умолчанию не позволяет в сигнатуру метода положить информацию о требуемом не-пустом значении, поэтому его можно описать в комментариях.

Теперь вы можете попытаться реализовать метод GetName методом с другой сигнатурой, где тип возврата указан, например, Stream. Однако компилятор вам этого не позволит, поскольку это нарушение контракта:
class Impl : IMyIface {
    Stream IMyIface.GetName() { return File.Open("file.txt"); } // error
}


Однако вы можете вернуть из GetName пустое значение (null):
class Impl : IMyIface {
    string IMyIface.GetName() { return null; }
}
Это так же нарушение контракта, хотя этот код скомпилируется.

Однако, в .NET можно написать код так, что метод GetName будет возвращать объект отличного от string типа (например, Stream). Это будет нарушение контракта (ведь пользователь ожидает string), но код скомпилируется. Тем не менее, он некорректен и работать вряд ли будет. (Я могу про это написать отдельный пост, тема интересная)

Таким же образом ваше требование о вызове PropertyChanged из другого потока — это нарушение контракта. Для кода уровня фреймворка это непростительно.

Допустим, у меня есть вьюмодел с множеством свойств, пара из которых может устанавливаться как из UI, так и из фонового потока. Причем установка из UI потока делается намного чаще, чем из фонового. Что делать?
Решить проблему на уровне свойств, а не АОП фреймворка.

Насчет искусственного ограничения вообще не понял — не вижу проблем написать аспект, который будет диспатчить вызовы. Приведенные выше примеры аспектов — они не встроены во фреймворк, они могут быть написаны и заинжекчены с помощью него, со всеми нюансами, которые вам нужны.
Как я говорил, INPC — это не та проблема, которая должна решаться через аспекты. Даже если это аспекты на конкретно этот класс или конкретно это свойство. Гораздо проще, чище и безопаснее написать полную реализацию вручную, задействовав сниппет, если лень писать повторяющиеся строчки.
Как я понял, от этого монстр-объекта предлагается наследоваться.
Это не всегда удобно, т.к. у viewmodel уже может быть другой предок.
AOP как раз позволяет внедрить код, не прибегая к наследованию.
И ничто не мешает внедрить же ту же реализацию с диспетчером.
А еще проще, пишется Snippet и при написании свойств используем его:
Snippet
<?xml version="1.0" encoding="utf-8" ?>
<CodeSnippets  xmlns="http://schemas.microsoft.com/VisualStudio/2005/CodeSnippet">
  <CodeSnippet Format="1.0.0">
    <Header>
      <Title>Define a Property for SL MVVM</Title>
      <Shortcut>propsl</Shortcut>
      <Description>Code snippet for a property using INotifyPropertyChanged. Property have comment</Description>
      <Author>al</Author>
      <SnippetTypes>
        <SnippetType>Expansion</SnippetType>
      </SnippetTypes>
    </Header>
    <Snippet>
      <Declarations>
        <Literal>
          <ID>myField</ID>
          <ToolTip>Field Name</ToolTip>
          <Default>myField</Default>
        </Literal>
        <Literal>
          <ID>type</ID>
          <ToolTip>Property Type</ToolTip>
          <Default>int</Default>
        </Literal>
        <Literal>
          <ID>property</ID>
          <ToolTip>Property Name</ToolTip>
          <Default>MyProperty</Default>
        </Literal>
        <Literal>
          <ID>comment</ID>
          <ToolTip>Commentary</ToolTip>
          <Default>Коментарий свойства</Default>
        </Literal>
      </Declarations>
      <Code Language="csharp">
        <![CDATA[
/// <summary>
/// $comment$
/// </summary>
private $type$ _$myField$ = null;

/// <summary>
/// Возвращает и задает свойство: $comment$
/// </summary>
public $type$ $property$
{
    get
    {
        return _$myField$;
    }
    set
    {
        if (_$myField$ != value)
        {
            _$myField$ = value;
            RaisePropertyChanged("$property$");
        }
    }
}
$end$]]>
      </Code>
    </Snippet>
  </CodeSnippet>
</CodeSnippets>


Только потом, при переименовании свойства, RaisePropertyChanged с большой вероятностью будет нотифицировать по старому имени (если сам разработчик или какой-нибудь решарпер не заметит)
Нашли к чему придраться :)
Проблему MagicString снипетами никак не решить, увы :)
nameof, CallerMemberNameAttribute?
nameof в нерелизной версии C# :)
CallelMemberName — такая же магия, как и аспекты, только прибитая гвоздями к компилятору. Зачем инжектить CallerMemberName, когда можно всю реализацию целиком?
Это не придирка, а констатация проблем, неоднократно случавшихся на практике.
Серебрянной пули нет, поэтому все чекины у нас, например, проходят через CodeReview, да и тестирование никто не отменял. Я, если честно, не помню ошибок связанных с тем сценарием который вы привели.
Я, к сожалению, помню проблемы из практики на реально большом проекте, когда после переименования возникали подобные проблемы.
Если сидеть на .NET 3.5, то безусловно. Вернем все взад!
Автоматизация INPC — всего лишь один из примеров использования. Повторюсь, что Aspect Injector — универсальный фреймворк, а не «еще один NotifyPropertyWeaver».
Почитал комментарии — даже обидно за вас.
Вы про аспекты, про открытую и бесплатную альтернативу postsharp… а вам в пример тычут.
Хотя это лишь пример, не хуже чем на главной странице postsharp. Даже заголовок статьи не обещает решения конкретно INotifyPropertyChanged-ых «проблем».
PS: Время назад делал «дописыватель» IL для экономного логирования (да, не хотел проксировать и покупать постшарп). И говорю отдельное и большое спасибо Вам — мне не хватало именно этого.
Писал когда-то свой велосипед, тоже на Mono.Cecil.
Вместо классических inject-точек Before/After я предпочёл сделать обёртку.

То есть, программист аспекта предоставляет метод
object OnCall(Action<object[], object> originalMethod, object[] params, SomeContext ctx);

Который получает управление вместо оригинального метода. Метод OnCall может проверить и/или модифицировать параметры, вызвать исходный метод через originalMethod(params), залогировать/модифицировать результат, обернуть вызов в try-except, вызвать метод несколько раз.

В ctx можно найти название перехваченного метода, класса и т.п.

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

Минус — некоторая потеря времени на боксинг/анбоксинг аргументов и результата, если они значимые типы и расход времени на создание object[] params
UPD: Func<object[], object> конечно, а не Action
Хорошая идея, спасибо! Но сама врезка получится менее тривиальной. Скорее всего нужно будет копировать тело функции под другим именем (фактически переименовать исходную функцию), а вместо тела исходной добавить тело адвайса. Возможно мы добавим такую фичу, например в виде типа точки врезки InjectionPoints.Around.
Идей было реализовано множество. Если интересно, могу что-нибудь ещё подкинуть.
1. Удобно создавать собственные aspect-атрибуты (наследуя от базового Aspect), т.е. вместо длинного
[Aspect(typeof(SoapExceptionConvertorAspect))]
иметь более лаконичное
[SoapExceptionConvertor]
(такой аспект может ловить любой exception метода и перевыкидывать SoapException, помещая исходный exception в InnerException. Если выпустить исключение наружу из веб-сервиса, клиент увидит ошибку 500, а SoapException передаётся клиенту с Message и т.п.).
2. Удобно своим аспектам передавать параметры.
[Transaction(500)]
или
[Transaction(Timeout = 500, Isolation = Levels.Snapshot)]
Второй вариант синтаксиса руки не дошли реализовать.

Враппер экземпляр класса атрибута, инициализирует его параметрами и передаёт в метод OnCall. В данном примере —
OnCall(..., new TransactionAttribute(500), ...);
OnCall(..., new TransactionAttribute { Timeout = 500, Isolation = Levels.Snapshot }, ...);
3. Удобно аспект-атрибуту передавать уточняющие атрибуты

Пример. Потребовалось удобно создавать распределённые транзакции, чтобы транзация коммитилась при выходе и роллбечилась при исключении.
Синтаксис выбран такой:
        [Transaction(500)]
        [SalesConnection]
        [SalaryConnection(true)]
        string SomeMethod(int a, int b)


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

Аспект-трансформер генерит такое тело:
        string SomeMethod(int a, int b)
        {
            // созданная обёртка содержит:
            var attrs = new WrapperParameter[]
                       {
                           new TransactionAttribute(500),
                           new SalesConnectionAttribute(),
                           new SalaryConnectionAttribute(true),
                       };
            return (string)TransactionAttribute.OnCall(originalBody, this, new object[] { a, b }, attrs, methodStaticInfo);
        }


Нужно указать, какие параметры относятся к какому аспекту. Я использую такой синтаксис:

Описание инфраструктуры
    
    public class AspectAttribute : Attribute
    {
    }

    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Struct)]
    public class WrapperParameter : AspectAttribute
    {
    }

    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Struct)]
    public class MethodWrapperAttribute : WrapperParameter
    {
        public static object OnCall(Func<object[], object> originalMethod, object thisClass, object[] originalMethodParams,
            WrapperParameter[] Attrs, OriginalMethodStaticInfo methodInfo) { return null; }
    }

    [AttributeUsage(AttributeTargets.Class, AllowMultiple = true)]
    public class LinkToWrapperAttribute : AspectAttribute
    {
        public LinkToWrapperAttribute(Type wrapperType) { }
    }

Прикладная часть
    
    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Struct)]
    public class TransactionAttribute : MethodWrapperAttribute
    {
        // 'new' позволяет понять, что сигнатура совпала с эталоном в предке. 
        // не совпала - решарпер слово 'new' выделяет как избыточное
        public static new object OnCall(Func<object[], object> originalMethod, object thisClass, object[] originalMethodParams, WrapperParameter[] attrParams, OriginalMethodStaticInfo methodInfo)
        {
            throw new NotImplementedException();
        }

        public TransactionAttribute()
        {
        }
        public TransactionAttribute(int timeout)
        {
            Timeout = timeout;
        }
        internal int Timeout { get; set; }
    }

    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Struct, Inherited = false)]
    [LinkToWrapper(typeof(TransactionAttribute))]
    public abstract class BaseConnectionAttribute : WrapperParameter
    {
        public abstract IConnection GetConnection();
    }

    public class SalesConnectionAttribute : BaseConnectionAttribute
    {
        public override IConnection GetConnection()
        {
            throw new NotImplementedException();
        }
    }

    public class SalaryConnectionAttribute : BaseConnectionAttribute
    {
        public SalaryConnectionAttribute(bool IsReadOnly)
        {
        }
        public override IConnection GetConnection()
        {
            throw new NotImplementedException();
        }
    }

4. Не удержался обработку INPC внести внутрь aspect-weaver-а, хотя это выглядит как прикладной уровень.
Зато это позволило разметить зависимости, и классическую портянку
Скрытый текст
    public class WareVM : INotifyPropertyChanged
    {
        private decimal _qty;
        private decimal _price;

        public decimal Qty
        {
            get { return _qty; }
            set
            {
                if (value == _qty) return;
                _qty = value;
                OnPropertyChanged();
                OnPropertyChanged("Sum");
            }
        }

        public decimal Price
        {
            get { return _price; }
            set
            {
                if (value == _price) return;
                _price = value;
                OnPropertyChanged();
                OnPropertyChanged("Sum");
            }
        }

        public decimal Sum { get { return Qty*Price; } }

        public event PropertyChangedEventHandler PropertyChanged;

        [NotifyPropertyChangedInvocator]
        protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
        {
            PropertyChangedEventHandler handler = PropertyChanged;
            if (handler != null) handler(this, new PropertyChangedEventArgs(propertyName));
        }
    }

можно записать так:
Скрытый текст
    [ObservableObject]
    public class WareVM
    {
        public decimal Qty { get; set; }
        public decimal Price { get; set; }
        [DependOn("Qty", "Price")]
        public decimal Sum { get { return Qty*Price; } }
    }

или так, на выбор:
Скрытый текст
    [ObservableObject]
    public class WareVM
    {
        [AlsoFires("Sum")]
        public decimal Qty { get; set; }
        [AlsoFires("Sum")]
        public decimal Price { get; set; }
        public decimal Sum { get { return Qty*Price; } }
    }

Код на выходе не отличается от референсной портянки
Большое спасибо! Это интересно, будем «переваривать» )
Скорее всего нужно будет копировать тело функции под другим именем (фактически переименовать исходную функцию), а вместо тела исходной добавить тело адвайса

Да, переименование исходного метода и создание нового под его имя (в cecil меняются местами body, чтобы не менять ссылки из других методов)
ref- и out-параметры тоже несложно сделать, но про них надо не забыть.
Ещё generics доставляют некоторые проблемы.
Сложно было победить generic-классы, а методы с generic-параметрами я и вовсе не осилил.
В решарпере есть удобный хелпер, преобразующий свойство в нотифицирующее. Зачем велосипед лепить… Можно подумать, осталось много тех, кто его не использует.
Но ведь это, по сути, копипаста, хоть и автоматизированная.

В-общем, всё зависит от программиста.
У каждого своё представление об идеальном коде. Кто-то предпочтёт маленькие повторяющиеся кусочки оформить в функции, а кто-то — оставить, как есть. При просмотре кода переходить из-за двух операторов в тело ф-ции может быть дольше, чем если бы эти операторы были просто записаны на месте вызова.
Sign up to leave a comment.

Articles