Strongly typed INPC

34 views
Skip to first unread message

Arild Fines

unread,
Aug 31, 2010, 3:47:58 PM8/31/10
to ReactiveXaml mailing list
Great library from what I've seen and read so far, although I haven't
had all that much time to dig into it yet.

What I'm wondering is if you've considered adding support for strongly
typed INPC notifications (and in other places where property names are
used as strings) in the same way that f.ex Caliburn Micro does it,
with expression trees and lambdas?

The literal strings kinda irk me a bit...

Paul Betts

unread,
Aug 31, 2010, 11:53:53 PM8/31/10
to reacti...@googlegroups.com
I've definitely considered that (especially to fix the abomination
that is RaiseAndSetIfChanged), I just haven't fully wrapped my head
around writing the code to walk the expression trees yet. Definitely
in the pipeline, I'll file a Github issue.

--
Paul Betts <pa...@paulbetts.org>

Oren Novotny

unread,
Sep 7, 2010, 4:22:21 PM9/7/10
to ReactiveXaml mailing list
Have you seen this:
http://www.ingebrigtsen.info/post/2008/12/11/INotifyPropertyChanged-revisited.aspx

I use that all the time in my view model base class and it's really
easy.


In the view model base:

protected void OnPropertyChanged(Expression<Func<object>> expression)
{
PropertyChanged.Notify(expression);
}

[field: NonSerialized]
public event PropertyChangedEventHandler PropertyChanged;

Then to raise a property:

public bool MyProp
{
get { return _myProp; }
private set
{
_myProp = value;
OnPropertyChanged(() => MyProp);
}
}

If you need the property name itself for the observable, then I'm sure
you can modify the Notify() extension method or add an override to get
it as an out parameter.

BTW, great stuff so far. One small nitpick...could you please update
your code style to follow the standard .net conventions of PascalCase?

Thanks!

On Aug 31, 11:53 pm, Paul Betts <paul.be...@gmail.com> wrote:
> I've definitely considered that (especially to fix the abomination
> that is RaiseAndSetIfChanged), I just haven't fully wrapped my head
> around writing the code to walk the expression trees yet. Definitely
> in the pipeline, I'll file a Github issue.
>
> --
> Paul Betts <p...@paulbetts.org>
>
>
>
> On Tue, Aug 31, 2010 at 12:47 PM, Arild Fines <arild.fi...@gmail.com> wrote:
> > Great library from what I've seen and read so far, although I haven't
> > had all that much time to dig into it yet.
>
> > What I'm wondering is if you've considered adding support for strongly
> > typed INPC notifications (and in other places where property names are
> > used as strings) in the same way that f.ex Caliburn Micro does it,
> > with expression trees and lambdas?
>
> > The literal strings kinda irk me a bit...- Hide quoted text -
>
> - Show quoted text -

Paul Betts

unread,
Sep 8, 2010, 2:50:13 PM9/8/10
to ReactiveXaml mailing list
This actually looks pretty similar to what I imagine, though mine is a
bit more complex - the syntax I'd like to use is:

string _SomeProperty;
public string SomeProperty {
get { return _SomeProperty; }
set { RaiseAndSet(x => x.SomeProperty = value); }
}

The trick with this function is, that you have to make it an extension
method, then actually reparse the expression into a getter, because
the order of the function has to be:

1. Check for equality (Here, we rewrite the setter AST to be an
equality test then execute it)
2. Set the field (Dig through the AST to find the property name, use
reflection to set the backing field)
3. Raise property changed

If you switch #2 and #3, your notification doesn't actually work since
the framework will end up reading out the old value. Another way it
could look is:

set { RaiseAndSet(x => x.SomeProperty, value); }

Thoughts? Which one is more appealing?

--
Paul Betts <pa...@paulbetts.org>

On Sep 7, 1:22 pm, Oren Novotny <o...@novotny.org> wrote:
> Have you seen this:http://www.ingebrigtsen.info/post/2008/12/11/INotifyPropertyChanged-r...

Jörg Battermann

unread,
Sep 19, 2010, 6:14:04 AM9/19/10
to ReactiveXaml mailing list
Paul,

looks good! However, the strict convention regarding the backing field
naming is somewhat 'unpleasant'. StyleCop / Microsoft Design
Guidelines say to use pascal case for fields without '_' etc (http://
msdn.microsoft.com/en-us/library/ms229012.aspx). Making it
'configurable' or smart to look up the backing field of a property
unfortunately cannot be done with standard functionality (a small
helper library named Mono.Reflection (http://evain.net/blog/articles/
2009/05/01/getting-the-field-backing-a-property-using-reflection)
could but would add another dependency) so I'd suggest going with the
official naming guidelines rather than a custom one.

-Jörg

On Sep 8, 8:50 pm, Paul Betts <paul.be...@gmail.com> wrote:
> This actually looks pretty similar to what I imagine, though mine is a
> bit more complex - the syntax I'd like to use is:
>
> string _SomeProperty;
> public string SomeProperty {
>     get { return _SomeProperty; }
>     set { RaiseAndSet(x => x.SomeProperty = value); }
>
> }
>
> The trick with this function is, that you have to make it an extension
> method, then actually reparse the expression into a getter, because
> the order of the function has to be:
>
> 1. Check for equality (Here, we rewrite the setter AST to be an
> equality test then execute it)
> 2. Set the field (Dig through the AST to find the property name, use
> reflection to set the backing field)
> 3. Raise property changed
>
> If you switch #2 and #3, your notification doesn't actually work since
> the framework will end up reading out the old value. Another way it
> could look is:
>
> set { RaiseAndSet(x => x.SomeProperty, value); }
>
> Thoughts? Which one is more appealing?
>
> --
> Paul Betts <p...@paulbetts.org>

Paul Betts

unread,
Sep 21, 2010, 2:30:38 PM9/21/10
to ReactiveXaml mailing list
Hmmm, that's true - what do you think of this compromise for v1, and
the next time I do a breaking change I'll switch it to use the
StyleCop way:

http://github.com/xpaulbettsx/ReactiveXaml/commit/98bb287e64529711a719c8c18feb2f1a06ddbfc1#L1L89

This gives you a way to override how we calculate a field name so you
can use any style you want, via a line at the top of your app (coding
in TextArea, ignore mistakes and dumb implementations):

// Property name "FooBarBaz" is backed by private field "fooBarBaz"
RxApp.GetFieldNameForPropertyFunc = new Func<string, string>(x =>
{ x[0] = Char.ToLower(x[0]); return x; } );

--
Paul Betts <pa...@paulbetts.org>

Jörg B.

unread,
Sep 25, 2010, 8:30:52 AM9/25/10
to ReactiveXaml mailing list
Paul,

good idea about using a method... works perfectly fine... nice!

Cheers
-J

On Sep 21, 8:30 pm, Paul Betts <paul.be...@gmail.com> wrote:
> Hmmm, that's true - what do you think of this compromise for v1, and
> the next time I do a breaking change I'll switch it to use the
> StyleCop way:
>
> http://github.com/xpaulbettsx/ReactiveXaml/commit/98bb287e64529711a71...
>
> This gives you a way to override how we calculate a field name so you
> can use any style you want, via a line at the top of your app (coding
> in TextArea, ignore mistakes and dumb implementations):
>
> // Property name "FooBarBaz" is backed by private field "fooBarBaz"
> RxApp.GetFieldNameForPropertyFunc = new Func<string, string>(x =>
> { x[0] = Char.ToLower(x[0]); return x; } );
>
> --
> Paul Betts <p...@paulbetts.org>

Oren Novotny

unread,
Sep 26, 2010, 5:56:40 PM9/26/10
to reacti...@googlegroups.com
What about something even simple and more performant -- using a ref parameter?

You'd call it something like RaiseAndSet(x => x.MyProp, ref _myPropStorage, value)

Then it won't matter what the backing field is named.

Also, one thing to keep in mind is that the expression tree parsing for getting the string value out of x => x.MyProp is very useful, but for properties that change very fast, there's a performance bottleneck in the Expression internals. I'd recommend having an overload that takes a string for those cases where a profiler deems a string faster than an expression. If a ref parameter is used for the storage, then that works fine as well.

Oren

Paul Betts

unread,
Sep 27, 2010, 1:30:49 PM9/27/10
to ReactiveXaml mailing list
Actually, RaiseAndSet is primarily a helper function - there is still
the RaisePropertyChanged method that simply takes the parameter name
as a string - if performance is a concern for certain properties,
writing the property setter by-hand is probably the fastest way

--
Paul Betts <pa...@paulbetts.org>

On Sep 26, 2:56 pm, Oren Novotny <o...@novotny.org> wrote:
> What about something even simple and more performant-- using a ref parameter?
>
> You'd call it somethinglike RaiseAndSet(x=> x.MyProp, ref _myPropStorage, value)
>
> Then it won't matter what the backing field is named.
>
> Also, onething to keep in mind is that the expressiontree parsing for getting the string value out of x => x.MyProp is very useful, but for properties that change very fast, there's a performance bottleneck in the Expression internals.  I'd recommend having an overload that takes a string forthose cases where a profilerdeems a string faster than an expression.  If aref parameteris used for the storage, then that works fine as well.  
>
> Oren
>
>
>
>
>
>
>
> > -----Original Message-----
> >From: reacti...@googlegroups.com
> > [mailto:reacti...@googlegroups.com] On Behalf Of Jörg B.
> > Sent: Saturday, September 25, 2010 8:31 AM
> > To: ReactiveXaml mailing list
> > Subject: Re: Strongly typed INPC
>
> > Paul,
>
> > good idea about using a method...works perfectlyfine... nice!
>
> > Cheers
> > -J
>
> > On Sep 21, 8:30 pm, Paul Betts <paul.be...@gmail.com> wrote:
> > > Hmmm,that's true - what do you think of this compromisefor v1, and
> > > the next time I do a breaking change I'll switch it to use the
> > > StyleCop way:
>
> >http://github.com/xpaulbettsx/ReactiveXaml/commit/98bb287e64529711a7
> > 1...
>
> > > This gives you a way to override how we calculate a field name so you
> > > can use any style you want, via aline at the top of your app (coding
> > > in TextArea, ignore mistakesand dumb implementations):
>
> > > // Property name "FooBarBaz" is backed by private field"fooBarBaz"
> > > RxApp.GetFieldNameForPropertyFunc = new Func<string, string>(x => {
> >> x[0] = Char.ToLower(x[0]); return x; } );
>
> > > --
> > > Paul Betts <p...@paulbetts.org>
>
> > > On Sep 19, 3:14 am, Jörg Battermann <j...@joergbattermann.com>wrote:
>
> > > > Paul,
>
> >> > looks good! However, the strict convention regarding the backing
> > > > field naming is somewhat 'unpleasant'. StyleCop / Microsoft Design
> > > > Guidelines say to use pascal casefor fields without'_' etc
> > > > (http:// msdn.microsoft.com/en-us/library/ms229012.aspx). Making it
> > > > 'configurable' or smart to look up the backing field of a property
> > > > unfortunately cannot be done with standard functionality (a small
> > > > helper librarynamed Mono.Reflection
> > > > (http://evain.net/blog/articles/
> > > > 2009/05/01/getting-the-field-backing-a-property-using-reflection)
> > > > could but would add another dependency) so I'd suggest going with
> > > > the official naming guidelines rather than acustom one.
>
> > > > -Jörg
>
> > > > On Sep 8, 8:50 pm, Paul Betts <paul.be...@gmail.com> wrote:
>
> > > > > Thisactuallylooks pretty similar to what I imagine, though mine
> >> > > is a bit more complex - the syntax I'd like to use is:
>
> > > > > string _SomeProperty;
> > > > >public string SomeProperty {
> > >> >     get { return _SomeProperty;}
> > > > >     set{ RaiseAndSet(x=> x.SomeProperty = value); }
>
> > > > > }
>
> > > > >The trick with this functionis, that you haveto make it an
> > > > > extension method,then actuallyreparse the expression into a
> >> > > getter, becausethe order of the function has to be:
>
> > >> > 1. Check for equality(Here, we rewrite the setter AST tobe an
> >> > > equalitytest then execute it) 2. Set thefield (Dig throughthe
> > > >> AST to find the propertyname, use reflection to set the backing
> > > > > field) 3. Raise property changed
>
> > > > > If you switch #2 and #3, your notification doesn't actually work
> > > > > since the framework will end up reading out the old value. Another
> > > > >way it could look is:
>
> > > > > set { RaiseAndSet(x=> x.SomeProperty,value); }
>
> > > > > Thoughts? Which one is more appealing?
>
> > > > > --
> > > > > Paul Betts...@paulbetts.org>
>
> > > > > On Sep 7, 1:22 pm, Oren Novotny <o...@novotny.org> wrote:
>
> > > > > > Haveyou seen
> > this:http://www.ingebrigtsen.info/post/2008/12/11/INotifyPropertyChange
> >d-r...
>
> > > > > > I use that all the time in my view model base class and it's
> > > > > > really easy.
>
> >> > > > In the  view model base:
>
> > > > > > protected voidOnPropertyChanged(Expression>
> > > > > > expression)
> > > > > >     {
> > > > >>       PropertyChanged.Notify(expression);
> >> > > >     }
>
> > > > > >     [field:NonSerialized]
> > > > > >     public event PropertyChangedEventHandler PropertyChanged;
>
> > > > > > Then to raise aproperty:
>
> > > > > > public bool MyProp
> > > > > >     {
> > > > > >       get { return _myProp; }
> > > > > >       private set
> > > > > >       {
> > > > > >         _myProp = value;
> > > > > >        OnPropertyChanged(()=> MyProp);
> > > >> >       }
> > > > > >     }
>
> > > >> > If you need the propertyname itself for the observable, then
> > > > > > I'm sure you can modify the Notify()extensionmethod or add an
> > > > > > override to get it as an out parameter.
>
> > > >> > BTW, great stuff so far.  One small nitpick...could you please
> > > > >> update your code style to follow the standard.net conventionsof
> > PascalCase?
>
> > > > > > Thanks!
>
> > > > > > On Aug 31, 11:53 pm, Paul Betts <paul.be...@gmail.com> wrote:
>
> > > > > > > I've definitely considered that (especially to fix the
> > > > > > >abominationthat is RaiseAndSetIfChanged), I justhaven't
> > >> > > > fully wrapped my head around writingthe code to walk the
> > > > >> > expression trees yet. Definitelyin the pipeline, I'llfile a Github
> > issue.
>
> > > > > > > --
> > > > > > > Paul Betts <p...@paulbetts.org>
>
> > > > > > > On Tue, Aug 31, 2010 at 12:47 PM, Arild Fines <arild.fi...@gmail.com>
> > wrote:
> > > > >> > > Great libraryfrom what I've seenand read so far, although
> > > > > > > > I haven't had all that much time todig into it yet.
>
> > > > > > > > What I'mwonderingis if you've considered adding support
> > >> > > > > for stronglytyped INPC notifications (and in other places
> > > > > > >> where property names are used as strings)in the same way
> > > > > > > > that f.ex CaliburnMicrodoes it, with expressiontrees and
> > lambdas?
>
> > > > > > > > The literalstrings kinda irkme a bit...-Hidequoted text
Reply all
Reply to author
Forward
0 new messages