Opinion wanted: Intercepting changes to lists/collections

Question

Although BindingList<T> and ObservableCollection<T> provide mechanisms to detect list changes, they don't support mechanisms to detect/intercept changes before they happen.

I'm writing a couple of interfaces to support this, but I want to canvas your opinion.

Option 1: Lists raise events for each type of action

Here, consumers might write code like this:

public class Order : Entity
    {
        public Order()
        {
            this.OrderItems = new List<OrderItem>();
            this.OrderItems.InsertingItem += new ListChangingEventHandler<OrderItem>(OrderItems_InsertingItem);
            this.OrderItems.SettingItem += new ListChangingEventHandler<OrderItem>(OrderItems_SettingItem);
            this.OrderItems.RemovingItem += new ListChangingEventHandler<OrderItem>(OrderItems_RemovingItem);
        }

        virtual public List<OrderItem> OrderItems { get; internal set; }

        void OrderItems_InsertingItem(object sender, IOperationEventArgs<OrderItem> e)
        {
            if (!validationPasses)
            {
                e.Cancel = true;
                return;
            }

            e.Item.Parent = this;
        }

        void OrderItems_SettingItem(object sender, IOperationEventArgs<OrderItem> e)
        {
            if (!validationPasses)
            {
                e.Cancel = true;
                return;
            }

            e.Item.Parent = this;
        }

        void OrderItems_RemovingItem(object sender, IOperationEventArgs<OrderItem> e)
        {
            if (!validationPasses)
            {
                e.Cancel = true;
                return;
            }

            e.Item.Parent = null;
        }

    }

Option 2: Lists raise a single event, and the action is determined from the event args

Here, consumers might write code like this:

public class Order : Entity
    {
        public Order()
        {
            this.OrderItems = new List<OrderItem>();
            this.OrderItems.ListChanging += new ListChangingEventHandler<OrderItem>(OrderItems_ListChanging);
        }

        virtual public List<OrderItem> OrderItems { get; internal set; }

        void OrderItems_ListChanging(object sender, IOperationEventArgs<OrderItem> e)
        {
            switch (e.Action)
            {
                case ListChangingType.Inserting:
                case ListChangingType.Setting:
                    if (validationPasses)
                    {
                        e.Item.Parent = this;
                    }
                    else
                    {
                        e.Cancel = true;
                    }
                    break;

                case ListChangingType.Removing:
                    if (validationPasses)
                    {
                        e.Item.Parent = null;
                    }
                    else
                    {
                        e.Cancel = true;
                    } 
                    break;
            }
        }

    }

Background: I'm writing a set of general purpose interfaces/classes that represent the core components of DDD, and I'm making the source code available (hence the need to create friendly interfaces).

This question is about making the interface as cohesive as possible, so that consumers can derive and implement their own collections without losing the core semantics.

PS: Please don't suggest using AddXYZ() and RemoveXYZ() methods for each list, because I've already discounted that idea.

PPS: I must include developers using .NET 2.0 :)

---

Related question.

Hydric

In the first case you can certainly use the same handler for InsertingItem and SettingItem – Dredi

Answer 1

I would suggest creating something that parallels the ObservableCollection<T> where appropriate. Specifically, I would suggest following the existing techniques for notification of change of collection. Something like:

class MyObservableCollection<T> 
    : INotifyPropertyChanging,   // Already exists
      INotifyPropertyChanged,    // Already exists
      INotifyCollectionChanging, // You'll have to create this (based on INotifyCollectionChanged)
      INotifyCollectionChanged   // Already exists
{ }

This will follow established patterns so that clients are already familiar with the exposed interfaces-- three of the interfaces already exist. The use of existing interfaces will also allow more proper interaction with other already existing .NET technologies, such as WPF (which binds against the INotifyPropertyChanged and INotifyCollectionChanged interfaces.)

I would expect the INotifyCollectionChanged interface to look something like:

public interface INotifyCollectionChanged
{
    event CollectionChangingEventHandler CollectionChanging;
}

public delegate void CollectionChangingEventHandler(
    object source, 
    CollectionChangingEventArgs e
);

/// <remarks>  This should parallel CollectionChangedEventArgs.  the same
/// information should be passed to that event. </remarks>
public class CollectionChangingEventArgs : EventArgs
{
    // appropriate .ctors here

    public NotifyCollectionChangedAction Action { get; private set; }

    public IList NewItems { get; private set; }

    public int NewStartingIndex { get; private set; }

    public IList OldItems { get; private set; }

    public int OldStartingIndex { get; private set; }
}

If you wish to add cancellation support, simply add a writable bool Cancel property to CollectionChangingEventArgs that the collection will read to determine whether to execute the change that's about to occur.

I suppose this falls under your Option 2. This is the way to go because, to interoperate properly with other .net technologies that monitor changing collections, you're going to have to implement it anyway for INotifyCollectionChanged. This will definitely follow the policy of "Least Surprise" in your interface.

Pahl

Thanks Greg. The "Princial of Least Suprise" was definitely the reason for option 2. In my quest to retain .NET 2.0 compatibility, I'd have to use the ListChanged event (INotifyCollectionChanged comes with .NET 3.0). ListChanged exposes the ListChangedType enum, which doesn't exactly fit my requirements. By adding more interfaces/enums to the mix, I fear that developers would be alienated by my library :( – Hydric

Answer 2

I would recomend seperate events. It seems more clear to me.

EDIT:

You might want to cosider a before and after event such as Inserting,Inserted or as the VB guys have it BeforeInsert, AfterInsert. This will give the user more flexability.

Quartus

You don't think it would be tedious, having to add all of those eventhandlers for every list in the entity? – Hydric

@Vijay: Event separation can ease the consumers of explicitly ignoring events of no interest to them. Good work you have there though! – Milligram

No, this will allow me to register listners to the specific events i require, without a generic switch block. – Quartus

@astander: The existing INotifyCollectionChanged already supports post changes - I don't want to bloat the interfaces too much. I'll definitely keep your suggestion in mind though. – Hydric

+1 - also like the idea with Before/After events. I would also make sure that the Before-event has a canceling mechanism, otherwise the Before-event makes little sense, to me at least. – Scathe

Have a single CollectionChanged event and a single CollectionChanging event. This is the most consistent approach. – Pahl

Answer 3

Have a look at this link, maybe that is what you are looking for, a Generic List based object that acts as a List but with built-in events such as BeforeItemAdded, ItemAdded, BeforeItemRemoved, ItemRemoved and ItemsCleared.

Hope this helps, Tom. :)

Tortious

I would recommend against this approach. It doesn't properly handle situations where a range of values change, for example, resulting in a reimplementation of something that the base has already exposed in a better way. – Pahl

Answer 4

Actually, you will be surprised how easily you can create a collection like that. Take a look at System.Collections.ObjectModel.Collection<T>. That is a class which is intended to be used for such things. It has a few virtual methods (one for every operation) which you can override and control very well.

I would recommend Option 1, since it is more clear and straightforward.

Here is an example which you can use for such purposes:

using System;
using System.Collections.ObjectModel;
using System.Collections.Generic;
using System.Linq;

namespace TestGround
{
    public class MyCollection<T> : Collection<T>
    {
        public class ListChangeEventArgs : EventArgs
        {
            public IEnumerable<T> ItemsInvolved { get; set;}

            public int? Index { get; set;}
        }

        public delegate void ListEventHandler(object sender, ListChangeEventArgs e);

        public event ListEventHandler Inserting;

        public event ListEventHandler Setting;

        public event ListEventHandler Clearing;

        public event ListEventHandler Removing;

        public MyCollection() : base() { }

        public MyCollection(IList<T> innerList) : base(innerList) { }

        protected override void ClearItems()
        {
            Clearing(this, new ListChangeEventArgs()
            {
                 Index = null,
                 ItemsInvolved = this.ToArray(),
            });
            base.ClearItems();
        }

        protected override void InsertItem(int index, T item)
        {
            Inserting(this, new ListChangeEventArgs()
            {
                Index = index,
                ItemsInvolved = new T[] { item },
            });
            base.InsertItem(index, item);
        }

        protected override void RemoveItem(int index)
        {
            Removing(this, new ListChangeEventArgs()
            {
                Index = index,
                ItemsInvolved = new T[] { this[index] },
            });
            base.RemoveItem(index);
        }

        protected override void SetItem(int index, T item)
        {
            Setting(this, new ListChangeEventArgs()
            {
                Index = index,
                ItemsInvolved = new T[] { item },
            });
            base.SetItem(index, item);
        }
    }
}

You could also modify the ListChangeEventArgs to have a bool property with the name "Cancel", and control wheter to do the change or not in the collection.

The after events could also be useful, if you need such functionality.

Of course, you won't have to use all events of every collections, or if it is really necessary, there may be other ways to solve the problem depending on why do you need this functionality.

EDIT:

If you really only want to validate the items and set their Parent property to an entity instance, you can actually write a collection which does exactly that, or something that generalizes the problem in another way. You could pass it a delegate which validates the item, and and another which tells it what to do when an item is added or removed.

For example, you can achieve this using the Action delegate.

You could consume it this way:

class Order : Entity
{
    public Order()
    {
        OrderItems = new MyCollection2<OrderItem>(
            //Validation action
            item => item.Name != null && item.Name.Length < 20,
            //Add action
            item => item.Parent = this,
            //Remove action
            item => item.Parent = null
        );
    }

    ...
}

The major benefit of this approach is that you don't have to bother with event handlers or delegates, beacuse all that you need can be written using lambda expressions, however if you need something more advanced, you can always use a real delegate instead of them.

This is an example of the collection:

public class MyCollection2<T> : Collection<T>
{
    public Func<T, bool> Validate { get; protected set; }

    public Action<T> AddAction { get; protected set; }

    public Action<T> RemoveAction { get; protected set; }

    public MyCollection2(Func<T, bool> validate, Action<T> add, Action<T> remove)
        : base()
    {
        Validate = Validate;
        AddAction = add;
        RemoveAction = remove;
    }

    protected override void ClearItems()
    {
        foreach (var item in this)
        {
            RemoveAction(item);
        }
        base.ClearItems();
    }

    protected override void InsertItem(int index, T item)
    {
        if (Validate(item))
        {
            AddAction(item);
            base.InsertItem(index, item);
        }
    }

    protected override void RemoveItem(int index)
    {
        RemoveAction(this[index]);
        base.RemoveItem(index);
    }

    protected override void SetItem(int index, T item)
    {
        if (Validate(item))
        {
            RemoveAction(this[index]);
            AddAction(item);
            base.SetItem(index, item);
        }
    }
}

For such purposes, I think this is the cleanest way to go.

Roxyroy

Thanks for the answer. In fact, my library already has base classes that implement the functionality. This question is about making the interface as cohesive as possible, so that consumers can derive and implement their own collections without losing the core semantics. – Hydric

I edited the post to add one more option for you, without all the events. This is a specialized approach which would spare you from having to wire up all the event handlers of every collections in every entity. – Roxyroy

But, if you want a generic solution, and you don't mind having to bother with all the event handlers, I think Option 1 is the right way to go. – Roxyroy