Wpf: Suggestion : Simplify dependency property registration

Created on 21 Dec 2019  ·  10Comments  ·  Source: dotnet/wpf

I've recently gotten back into WPF and now I'm relearning the fun of defining DependencyProperties.

Existing Behavior

Lets look at the current way of defining some DPs;

public class MyViewModel : DependencyObject
{
    public Brush Fill
    {
        get => (Brush)GetValue(FillProperty);
        set => SetValue(FillProperty, value);
    }

    public Brush Foreground
    {
        get => (Brush)GetValue(ForegroundProperty);
        set => SetValue(ForegroundProperty, value);
    }

    public string Text
    {
        get => (string)GetValue(TextProperty);
        set => SetValue(TextProperty, value);
    }

    public static readonly DependencyProperty FillProperty = DependencyProperty.Register(nameof(Fill), typeof(Brush), typeof(MyViewModel), new PropertyMetadata(Brushes.White));
    public static readonly DependencyProperty ForegroundProperty = DependencyProperty.Register(nameof(Foreground), typeof(Brush), typeof(MyViewModel), new PropertyMetadata(Brushes.Black));
    public static readonly DependencyProperty TextProperty = DependencyProperty.Register(nameof(Text), typeof(string), typeof(MyViewModel), new PropertyMetadata(""));
}

I always found the process clunky and very verbose. In particular, we have to keep repeating typeof(MyViewModel) on each declaration.
We also have to be careful of copy/paste errors on the property types as well as the default values passed into the property metadata.

Proposed Behavior

I tried to reduce this pain by defining two helper types.

public readonly struct DependencyProperty<T>
{
    public DependencyProperty(DependencyProperty property)
    {
        Property = property;
    }

    public DependencyProperty Property { get; }

    public static implicit operator DependencyProperty(DependencyProperty<T> dependencyProperty) 
        => dependencyProperty.Property;
} 
public class DependencyObject<TSelf> : DependencyObject
    where TSelf : DependencyObject<TSelf>
{
    public T Get<T>(DependencyProperty<T> property) => (T)GetValue(property.Property);

    public void Set<T>(DependencyProperty<T> property, T value) => SetValue(property.Property, value);

    public static DependencyProperty<T> RegisterProperty<T>(string name) 
        => new DependencyProperty<T>(DependencyProperty.Register(name, typeof(T), typeof(TSelf)));

    public static DependencyProperty<T> RegisterProperty<T>(string name, T defaultValue) 
        => new DependencyProperty<T>(DependencyProperty.Register(name, typeof(T), typeof(TSelf), new PropertyMetadata(defaultValue)));


    public static DependencyProperty<T> RegisterProperty<T>(Expression<Func<TSelf, T>> propertyExpression) 
        => RegisterProperty<T>(GetPropertyNameFromExpression(propertyExpression));

    public static DependencyProperty<T> RegisterProperty<T>(Expression<Func<TSelf, T>> propertyExpression, T defaultValue)
        => RegisterProperty(GetPropertyNameFromExpression(propertyExpression), defaultValue);

    private static string GetPropertyNameFromExpression<T>(Expression<Func<TSelf, T>> propertyExpression)
    {
       //sample implementation ommited for brevity...
    }
}

DependencyProperty<T> is a wrapper struct that associates a strong type with the underlying DependencyProperty.
DependencyObject<T> is a new type that you would derive from instead of directly from DependencyObject.
It has simpler helpers that make defining dependency easier and less error prone.

With these changes we can now define our view model like this:

public class MyViewModel : DependencyObject<MyViewModel>
{
    public Brush Fill
    {
        get => Get(FillProperty);
        set => Set(FillProperty, value);
    }

    public Brush Foreground
    {
        get => Get(ForegroundProperty);
        set => Set(ForegroundProperty, value);
    }

    public string Text
    {
        get => Get(TextProperty);
        set => Set(TextProperty, value);
    }

    public static readonly DependencyProperty<Brush> FillProperty = RegisterProperty<Brush>(nameof(Fill), Brushes.White);
    public static readonly DependencyProperty<Brush> ForegroundProperty = RegisterProperty<Brush>(nameof(Foreground), Brushes.Black);
    public static readonly DependencyProperty<string> TextProperty = RegisterProperty(nameof(Text), "");
}

There are two main benefits.

  • First, type correctness is enforced at compile time.
  • Second, the registration is simpler and easier to read.

Compare:

DependencyProperty.Register(nameof(Fill), typeof(Brush), typeof(MyViewModel), new PropertyMetadata(Brushes.White));

to

RegisterProperty<Brush>(nameof(Fill), Brushes.White);

Alternatively, we can also use a lambda to register the property like this

RegisterProperty(self => self.Fill, Brushes.White);

Drawbacks

I'm using the C# version of CRTP to make this work. It's perfectly legal syntax, but if you haven't seen it before it can be very confusing at first. But it saves me from having to always specify the owning type of the property.

This also my just be too little, too late and it might not be worth the added confusion.

issue-type-enhancement

Most helpful comment

I've hated dependency property registration for a long time and have written this to ease the pain: https://gist.github.com/zlatanov/8ed191d517d2a84529f66b73347cd7ca.

For it to work, I am (ab)using C# using static class feature and expression trees. Here is example:

``` c#
using static DependencyPropertyRegistrar;

namespace AcmeCorp
{
class PowerControl : UserControl
{
public static readonly DependencyProperty GreetingProperty =
RegisterProperty( x => x.Greeting ).OnChange( x => x.OnGreetingChanged );

    public string Greeting
    {
         get => (string)GetValue( GreetingProperty );
         set => SetValue( GreetingProperty, value );
    }

    protected virtual void OnGreetingChanged( string oldValue, string newValue )
    {
         // Even callbacks no longer need to be static
    }
}

}
```

The helper class works as well for read-only and attached properties. I don't think from usability point of view (API) it can get any easier.

All 10 comments

My 2¢. I would highly recommend trying to get rid of CRTP, as it is IMHO very un-C#-like. My suggested implementation would look like this instead:

DependencyProperty<TValue> RegisterProperty<TOwner, TValue>(string name, TValue defaultValue) {
    return new DependencyProperty<TValue>(
        DependencyProperty.Register(name, typeof(TValue), typeof(TOwner), new PropertyMetadata(defaultValue))
    );
}

// usage:
public class MyViewModel : DependencyObject {
    public static readonly DependencyProperty FillProperty = RegisterProperty<MyViewModel, Brush>(nameof(Fill), Brushes.White);
}

As for your concerns about accidentally mixing up the various types and names, have you seen Roslyn Analyzers? These are essentially custom compiler diagnostics (errors and/or warnings). They can be distributed in a NuGet package, either by themselves or alongside the code that they check. I would highly suggest creating a Roslyn Analyzer that checks to ensure that all of the parameters match instead. Here’s a tutorial on how you would write one.

Finally, please don’t forget that the idiom for WPF is to declare DependencyProperty instances in static readonly variables. This is important because it places the Register() calls into the static constructor for the enclosing class, which guarantees that the dependency properties will be registered before any code in the class that defines them is run. If you put them in static properties, then the properties will be registered lazily, when they are first accessed, which may cause problems. (The only reason why UWP/WinUI XAML uses static properties for these objects is because the underlying WinRT runtime doesn’t support public variables.) Hope this helps!

I figured CRTP would be a tough sell. I am also familiar with analyzers, I love them, but writing, maintaining and then asking the user to install them is a big ask compared to the two line change for CRTP 🤷‍♀️.

I thought about it a few different ways and the intermediate inheritable type feels the best to me.
You get the autocasted GetValue helper and the syntactic compactness of a single type argument.

I also wanted to avoid the double type parameter on the register method for a couple of reasons.

  1. You have to remember the order of the type params
  2. A big part of the ergonomics of this proposal to me was type inference on the method call.
  3. Even if one of the types could be inferred (such as TValue) you have to specify both always.
  4. It feels wasteful to have to repeat a method parameter over and over. I am using the intermediate DependencyObject<T> type to inform the compiler that all register calls will use the same argument.

The only reason i have Brush specified in my example is to prevent the type inference from being overly specific with SolidColorBrush. But note that the Text property is fully inferred as is the lambda syntax.

Lastly, I didn't realize the quirk, about static properties. I fact i forgot i was using them 😅.
But do they really initialize differently? The generated IL ends up with the same readonly backing field.

But do they really initialize differently?

@AlgorithmsAreCool are defining the DP as static properties, with compiler generated private readonly backing field (of undefined name). @wjk is declaring static field directly. I would be worried about breaking things switching from fields to properties, but that doesn't really affect the proposal.

I rarely had to declare a DP manually, you don't have to remember much when using snippets, and you can get most of it prefilled. I was occasionally bitten by the default value having different type than the property declared type and you quickly become aware of the problem, though I guess an extra generic overload would make it easier.

The problem with generic methods is that you need reflection to invoke them if you don't know the type at compile time. In the proposed way you also lose the ability to declare properties with different owner type than the declaring type. How does the performance between the current and the proposed version compare?

@miloush i've updated my code to not use static properties, they were not part of the proposal.

I'm not sure when you would need to dynamically register dependency properties. I though the whole point was to manually register them so the facade properties could efficiently access them.

In anycase, this proposal would not deprecate the existing Register methods, so if you needed an usual usage such as registering dependency properties for another class, then you could always just use the old methods. My intent was to cover the 90% use case, not all possible cases.

Runtime perf should be identical. There is technically an addition struct field access on property read/writes but the JIT should easily be able to make that disappear.

I would however expect that using the lambda syntax would incur additional cost at app startup since inspecting the expression tree isn't free. If you do that with hundreds of properties it could get expensive. But the nameof syntax should be identical to the traditional Register methods.

Lastly, call me biased, but i think my code reads easier, less noise on the register call.

I am for the spirit of proposal. Simplifying DP registration I think is definitely a good thing. But introducing an intermediate class is... a little bit rusty. While we're at at, why not replace nameof(Property) with some use of CallerMemberName attribute?

why not replace nameof(Property) with some use of CallerMemberName attribute

Because the registration is not called from the property context; the field has a different name. I am more annoyed about the attached properties not being able to take advantage of nameof.

the field has a different name

I mean, yes, but it is "MyNameProperty" - we can just remove "Property".
C# DependencyProperty Register([CallerMemberAttribute] string name = null) { name = name.Remove(name.Length - "Property".Length, "Property".Length); ... }
This is of course prototype code, since it doesn't handle the cases of manual name specification well.

I've hated dependency property registration for a long time and have written this to ease the pain: https://gist.github.com/zlatanov/8ed191d517d2a84529f66b73347cd7ca.

For it to work, I am (ab)using C# using static class feature and expression trees. Here is example:

``` c#
using static DependencyPropertyRegistrar;

namespace AcmeCorp
{
class PowerControl : UserControl
{
public static readonly DependencyProperty GreetingProperty =
RegisterProperty( x => x.Greeting ).OnChange( x => x.OnGreetingChanged );

    public string Greeting
    {
         get => (string)GetValue( GreetingProperty );
         set => SetValue( GreetingProperty, value );
    }

    protected virtual void OnGreetingChanged( string oldValue, string newValue )
    {
         // Even callbacks no longer need to be static
    }
}

}
```

The helper class works as well for read-only and attached properties. I don't think from usability point of view (API) it can get any easier.

@zlatanov Yep, i landed on almost the exact same idea with expression trees.

I figured CRTP would be a tough sell.

The original proposal also won't work when you have to define subclasses. It bakes the type of the owner into the superclass, so if you want to inherit and add a DP to your subclass you are screwed.

(Thats one of the usual reasons why CRTP is a bad choice for C#, even though .NET generics have the same syntax as C++ templates they work entirely differently and this is one of the breaking points, baking the CRTP into the superclass and exposing your helper class -an implementation detail- as public API is usually not the right thing to do.)

I see some answers suggest alternatives, considering the OP suggestion doesn't work this issue needs to be either reconciled with those suggestions (updating the top post) or close and reopen with a new proposal (linking back here if someone wants the history). Otherwise in a few posts it will be impossible to figure out which of the comments presents a working suggestion.

Was this page helpful?
0 / 5 - 0 ratings