WPF Custom Control with a Single ObservableCollection DependencyProperty

Question

I'm learning how to write WPF custom controls. My example, NameLister, exposes a single DependencyProperty, Names, of type ObservableCollection<string>, and displays all the names concatenated together into a single string in a TextBlock.

To test my control, I have a TestApp which binds a collection of three strings: "Larry", "Curly", and "Moe" to the Names property of my control. It then has a repeating timer which adds "Me" strings to its collection.

There are a few things I'm puzzled about:

1. This sure seems like a lot of code just for one DependencyProperty to which someone can bind an ObservableCollection.
2. Should I be using ObservableCollection, IEnumerable, or IList as the type for my Names DependencyProperty?
3. I made my custom control implement INotifyPropertyChanged. I did this because I use a normal property, DisplayString, bound to the TextBlock's Text property in the XAML. However, I've heard this is a bad idea (because my custom control is a view, not a view-model?)

TestApp

MainWindow.xaml

<Window x:Class="TestApp.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:sdwc="clr-namespace:NameListerWpfControl;assembly=NameListerWpfControl"
        xmlns:local="clr-namespace:TestApp"
        mc:Ignorable="d"
        Title="MainWindow" Height="450" Width="800" BorderBrush="#FF0010FF">
    <Window.DataContext>
        <local:ViewModel />
    </Window.DataContext>
    <Grid>
        <sdwc:NameLister Background="Red" Names="{Binding Stooges}" />
    </Grid>
</Window>

ViewModel.cs

using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.ComponentModel;

namespace TestApp
{
  public class ViewModel : INotifyPropertyChanged
  {
    public ObservableCollection<string> Stooges { get; set; } = new ObservableCollection<string> {"Larry", "Curly", "Moe"};

    public ViewModel()
    {
      var dispatcherTimer = new System.Windows.Threading.DispatcherTimer
      {
        Interval = new TimeSpan(0, 0, 2)
      };

      dispatcherTimer.Tick += delegate
      {
        Stooges.Add("Me");
        RaisePropertyChanged(nameof(Stooges));
      };
      dispatcherTimer.Start();
    }

    /// <summary>
    /// The property changed event
    /// </summary>
    public event PropertyChangedEventHandler PropertyChanged;

    /// <summary>
    /// Raises the property changed event
    /// </summary>
    /// <param name="property">The property name</param>
    public void RaisePropertyChanged(string property)
    {
      PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(property));
    }
  }
}

NameListerWpfControl

Themes/Generic.xaml

<ResourceDictionary
    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
    xmlns:local="clr-namespace:NameListerWpfControl">
    <Style TargetType="{x:Type local:NameLister}">
        <Setter Property="Template">
            <Setter.Value>
                <ControlTemplate TargetType="{x:Type local:NameLister}">
                    <TextBlock Text="{Binding RelativeSource={RelativeSource FindAncestor, 
                       AncestorType={x:Type local:NameLister}}, 
                       Path=DisplayString}"/>
                </ControlTemplate>
            </Setter.Value>
        </Setter>
    </Style>
</ResourceDictionary>

NameLister.cs

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.ComponentModel;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Windows;
using System.Windows.Controls;
using System.Windows.Data;
using System.Windows.Documents;
using System.Windows.Input;
using System.Windows.Media;
using System.Windows.Media.Imaging;
using System.Windows.Navigation;
using System.Windows.Shapes;

namespace NameListerWpfControl
{
  /// <summary>
  /// Lists names
  /// </summary>
  public class NameLister : Control, INotifyPropertyChanged
  {
    static NameLister()
    {
      DefaultStyleKeyProperty.OverrideMetadata(typeof(NameLister),
        new FrameworkPropertyMetadata(typeof(NameLister)));

    }

    /// <summary>
    /// The names displayed as a single string
    /// </summary>
    public string DisplayString => string.Join(", ", Names);


    /// <summary>
    /// Identifies the Names dependency property.
    /// </summary>
    public static readonly DependencyProperty NamesProperty =
      DependencyProperty.Register(
        "Names",
        typeof(ObservableCollection<string>),
        typeof(NameLister),
        new FrameworkPropertyMetadata(
          new ObservableCollection<string> (),
          OnNamesChanged, 
          CoerceStrings));



    /// <summary>
    /// Gets or sets the value assigned to the control.
    /// </summary>
    public ObservableCollection<string> Names
    {
      get => (ObservableCollection<string>)GetValue(NamesProperty);
      set => SetValue(NamesProperty, value);
    }

    private static object CoerceStrings(DependencyObject element, object value)
    {
      var newValue = (ObservableCollection<string>)value;
      // TODO: Actually do some coercion.
      return newValue;
    }

    private static void OnNamesChanged(DependencyObject obj,
      DependencyPropertyChangedEventArgs args)
    {
      var control = (NameLister)obj;

      control.RaisePropertyChanged(nameof(DisplayString));

      var action = new NotifyCollectionChangedEventHandler(
        (o, args2) =>
        {
          if (!(obj is NameLister lister)) return;

          lister.RaisePropertyChanged(nameof(DisplayString));
        });

      if (args.OldValue != null)
      {
        var coll = (INotifyCollectionChanged)args.OldValue;
        // Unsubscribe from CollectionChanged on the old collection
        coll.CollectionChanged -= action;
      }

      if (args.NewValue != null)
      {
        var coll = (INotifyCollectionChanged)args.NewValue;
        // Subscribe to CollectionChanged on the new collection
        coll.CollectionChanged += action;
      }

      var e = new RoutedPropertyChangedEventArgs<ObservableCollection<string>>(
        (ObservableCollection<string>)args.OldValue, 
        (ObservableCollection<string>)args.NewValue, 
        NamesChangedEvent);
      control.OnNamesChanged(e);
    }

    /// <summary>
    /// Identifies the NamesChanged routed event.
    /// </summary>
    public static readonly RoutedEvent NamesChangedEvent = 
      EventManager.RegisterRoutedEvent(
      "NamesChanged",
      RoutingStrategy.Bubble,
      typeof(RoutedPropertyChangedEventHandler<ObservableCollection<string>>),
      typeof(NameLister));

    /// <summary>
    /// Occurs when the Names property changes.
    /// </summary>
    public event RoutedPropertyChangedEventHandler<ObservableCollection<string>> NamesChanged
    {
      add => AddHandler(NamesChangedEvent, value);
      remove => RemoveHandler(NamesChangedEvent, value);
    }

    /// <summary>
    /// Raises the NamesChanged event.
    /// </summary>
    /// <param name="args">Arguments associated with the NamesChanged event.</param>
    protected virtual void OnNamesChanged(RoutedPropertyChangedEventArgs<ObservableCollection<string>> args)
    {
      RaiseEvent(args);
    }

    /// <summary>
    /// The property changed event
    /// </summary>
    public event PropertyChangedEventHandler PropertyChanged;

    /// <summary>
    /// Raises the property changed event
    /// </summary>
    /// <param name="property">The property name</param>
    public void RaisePropertyChanged(string property)
    {
      PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(property));
    }
  }
}

Answer 1

Without running the code myself, it seems you've done a pretty good job. There are just a couple things I would point out:

Names Default Value

Your dependency property for Names is declared as follows:

    public static readonly DependencyProperty NamesProperty =
      DependencyProperty.Register(
        "Names",
        typeof(ObservableCollection<string>),
        typeof(NameLister),
        new FrameworkPropertyMetadata(
          new ObservableCollection<string> (),
          OnNamesChanged, 
          CoerceStrings));

I'd like to draw your attention to the line new ObservableCollection<string> () and then show you this quote from Microsoft's documentation on Collection-Type Dependency Properties:

If your property is a reference type, the default value specified in dependency property metadata is not a default value per instance; instead it is a default value that applies to all instances of the type. Therefore you must be careful to not use the singular static collection defined by the collection property metadata as the working default value for newly created instances of your type. Instead, you must make sure that you deliberately set the collection value to a unique (instance) collection as part of your class constructor logic. Otherwise you will have created an unintentional singleton class.

That might be a bit to digest, so to simplify: If you set a default value for a collection in the dependency property metadata, that default collection will be shared by all instances of the class (like a static property). Instead, you need to leave the default in the metadata as null and set property in the class's constructor.

Names Type

To answer your question, I don't necessarily see a problem with you using ObservableCollection<string>, except that a popular general rule is "Return the most specific type, accept the most generic type".

If we're treating Names as sort of an "input property", then we would want it to be the most general type possible, which would probably be IEnumerable<string>. You would then check in OnNamesChanged whether the new value of the property implemented INotifyCollectionChanged and if so attach the appropriate event handler. If you take this approach, I would recommend taking inspiration from the ItemsControl class, specifically the ItemsSource property, since it does the same basic action of taking a bound collection and using it. You can find the actual source code for ItemsControl here which you can use to see how Microsoft went about implementing it.

On the other hand, implementing something like that takes considerably more work, and you might very well want to require an ObservableCollection<string> based how your control is intended to be used. In either case, leaving it as is should be fine as far as I'm concerned.