This project has moved and is read-only. For the latest updates, please go here.

FromPropertyChangePattern

Topics: Reactive (Rx)
May 10, 2011 at 6:05 PM


Observable2.FromPropertyChangedPattern(() => myobject.Testing);

One problem with this, is myobject.Testing might not support property change notification.

Would an extension method like this be useful?

IObservable<T> ToObservable<T>(this INotifiyPropertyChanged source, Expression<Func<INotifyPropertyChanged, T>> expression)

Thoughts?

May 10, 2011 at 6:06 PM

Usage

myObject.ToObservable(x => x.MyProperty)

May 10, 2011 at 6:26 PM

I don't understand, if myobject.Testing doesn't support property changed notifications then how would your proposed extension help?

The extension is named "Pattern" because it's only meant to support objects that use the common property-changed patterns.  Perhaps it would make sense to include a FromPropertyChanged extension in the same way that the Rx team included FromEvent along with FromEventPattern in the latest release, although I don't see how we could acquire property changed notifications from an object that doesn't support it in the first place.

Or am I missing something?

Related:  I put in a work item for a ContextBoundObject base class that would provide an alternative to implementing INotifyPropertyChanged manually, but it's still an opt-in feature.

http://rxx.codeplex.com/workitem/20663

- Dave

May 10, 2011 at 6:36 PM

Well this extension method wouldn't compile if myobject didn't implement INotifyPropertyChanged

May 10, 2011 at 6:38 PM

This doesn't offer any other functionality. But IMO it is qutie a nice API for INotifyPropertyChange objects

May 10, 2011 at 6:42 PM
Edited May 10, 2011 at 6:46 PM

I see, but FromPropertyChangedPattern doesn't require INotifyPropertyChanged.  It also supports the *Changed event pattern and DependencyProperty.

Is the concern that devs will assume that FromPropertyChangedPattern supports change events for all properties, even if the pattern wasn't actually implemented on the supplied object?

I don't think this needs to be a concern.  For one thing, it has "Pattern" in the name.  Secondly, it will throw at runtime.  I'm not sure it's really worth our time to ensure that it fails at compile time.  Instead, devs should learn about the API before using it.

What do you think?

May 10, 2011 at 6:48 PM
Edited May 10, 2011 at 6:48 PM

Should it be renamed to FromPropertyChangedPatterns?


May 10, 2011 at 7:01 PM

Lets leave it for now. If anything we could have an extension for INotifyPropertyChanged that calls through to this extension method right?

May 10, 2011 at 7:09 PM
Edited May 10, 2011 at 7:10 PM

We could, I'm just doubting its usefulness.  I think it might be confusing to people as it only provides a subset of the functionality of FromPropertyChangedPattern just so they'll get an error at compile time when they pass in an object that doesn't implement INotifyPropertyChanged.  Or are you thinking of another specific use case that I'm missing?

May 10, 2011 at 7:15 PM
Edited May 10, 2011 at 7:18 PM

Ah, I see what you mean now.  Sorry for being dense.

If you have a reference to an INotifyPropertyChanged instance, then you might as well have an extension that calls FromPropertyChangedPattern for you.

We could also have a similar extension for DependencyObject.

Maybe the extension should be called (edit:) PropertyChangedObservable?

May 10, 2011 at 7:21 PM
Edited May 10, 2011 at 7:22 PM

On second thought, it seems like it might just be clutter.  For one thing, we're not offering any additional functionality by doing it this way and there really doesn't seem to be any advantage, other than perhaps increased discoverability.

For example:

var fooChanged = Observable2.FromPropertyChangedPattern(() => obj.Foo);

-vs-

var fooChanged = obj.PropertyChangedObservable(() => obj.Foo);

Also, the latter requires two references to obj.  I think I prefer the former.

May 10, 2011 at 7:26 PM


>Also, the latter requires two references to obj. I think I prefer the former.

Have a look at my original API though... it would actually be;

var fooChanged = obj.PropertyChangedObservable(x => x.Foo);

May 10, 2011 at 7:27 PM

>other than perhaps increased discoverability.

Yes... its probably more about discoverability.

May 10, 2011 at 8:06 PM

> Have a look at my original API though

I noticed, but I find that style to be more confusing.  Or at least, it's no better than my example that specifies the obj variable directly since there's repetition in both.

In the original forum discussion the implemention of FromPropertyChangedPattern was similar, but since then I've elliminated the need for having to bind the object in the expression function for two reasons:

  1. Simplify the API.
  2. Support chained field/property syntax.  For example:

    var fooChanged = Observable2.FromPropertyChangedPattern(() => obj.Foo.Bar.Baz);

    The recommended overload of FromPropertyChangedPattern automatically detects the object returned by Bar and uses it to acquire property changed notifications for Baz.

I included the legacy syntax anyway as an overload of FromPropertyChangedPattern, but perhaps we should remove it entirely.  It seems to me that adding this extension to INotifyPropertyChanged will just add some confusion and a subset of the functionality that FromPropertyChangedPattern offers, so perhaps we should just stick with one recommended implementation.

Still though, is FromPropertyChangedPattern really that complicated to use now?  I'm not sure it's worth our time to add this feature since there's very little value in it, if any.