Search code examples
c#.netobservablerx.net

Wrap Observable.FromEventPattern with Observable.Create


Which is the difference between this (taken from RxCookbook on GitHub):

public static IObservable<TProperty> OnPropertyChanges<T, TProperty>(this T source, Expression<Func<T, TProperty>> property)
    where T : INotifyPropertyChanged
{
    return  Observable.Create<TProperty>(o=>
    {
        var propertyName = property.GetPropertyInfo().Name;
        var propertySelector = property.Compile();

        return Observable.FromEventPattern<PropertyChangedEventHandler, PropertyChangedEventArgs>(
                        handler => handler.Invoke,
                        h => source.PropertyChanged += h,
                        h => source.PropertyChanged -= h)
                    .Where(e => e.EventArgs.PropertyName == propertyName)
                    .Select(e => propertySelector(source))
                    .Subscribe(o);
    });
}

And this (written by myself):

public static IObservable<TProperty> OnPropertyChanges<T, TProperty>(this T source, Expression<Func<T, TProperty>> property)
    where T : INotifyPropertyChanged
{
    var propertyName = property.GetPropertyInfo().Name;
    var propertySelector = property.Compile();

    return Observable.FromEventPattern<PropertyChangedEventHandler, PropertyChangedEventArgs>(
                    handler => handler.Invoke,
                    h => source.PropertyChanged += h,
                    h => source.PropertyChanged -= h)
                .Where(e => e.EventArgs.PropertyName == propertyName)
                .Select(e => propertySelector(source));
}

I think that in the second block of code, propertyName and propertySelector will be evaluated when OnPropertyChanges is called and in the first block these variables will be evaluated every time someone subscribes to the observable. However, I can't figure out if/why the first block is preferrable to the second one, and why did the author of the first block decided to use Observable.Create.


Solution

  • Answer from the author of the first block of code on GitHub:

    My stance on this is that calling a method that returns an IObservable should do nothing. It is the subscription to that returned value that should invoke any side effects or processing.

    If a subscription cost needs to be amortised across numerous subscriptions then the various multicast options in Rx should be applied.

    In this specific case, the argument is weak, however it then muddies the pattern and opens the door for other methods to "do work" without a subscription. I see this as a very common mistake and a source of race conditions and leaking resources (think opening a connection, starting a timer etc)