1

Two classes:

    public class ObservableCollection<T>
    {
        public virtual event NotifyCollectionChangedEventHandler CollectionChanged;

        protected virtual void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
        {
            CollectionChanged.Invoke(this, e);
        }
    }

    public class DerivedObservableCollection<T> : ObservableCollection<T>
    {
        private bool _SuppressNotification;

        public override event NotifyCollectionChangedEventHandler CollectionChanged;

        protected virtual void OnCollectionChangedMultiItem(
            NotifyCollectionChangedEventArgs e)
        {
            NotifyCollectionChangedEventHandler handlers = this.CollectionChanged;
            if (handlers != null)
            {
                foreach (NotifyCollectionChangedEventHandler handler in 
                    handlers.GetInvocationList())
                {
                    if (handler.Target is CollectionView)
                        ((CollectionView)handler.Target).Refresh();
                    else
                        handler(this, e);
                }
            }
        }

        protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
        {
            if (!_SuppressNotification)
            {
                base.OnCollectionChanged(e);//<------is it usefull?------------
                if (CollectionChanged != null)
                    CollectionChanged.Invoke(this, e);
            }
        }
    }

Could one subscribe on ObservableCollection.CollectionChanged in DerivedObservableCollection instance (outside DerivedObservableCollection) and how? Or Is it usefull to call :

base.OnCollectionChanged(e);

?

Actually, I faced this problem with System.Collections.ObjectModel.ObservableCollection and this answer.

Кое Кто
  • 445
  • 5
  • 9
  • 1
    I haven't actually tested, but it looks like the event would need to be `vitual` in order to be overridden. You're not getting a compiler error? Maybe relevant? https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/events/how-to-raise-base-class-events-in-derived-classes – itsme86 Jul 24 '18 at 17:19
  • @itsme86 Thanks, fixed. – Кое Кто Jul 24 '18 at 17:33
  • 3
    @itsme86 The link you provided clearly states: "Do not declare virtual events in a base class and override them in a derived class. The C# compiler does not handle these correctly and it is unpredictable whether a subscriber to the derived event will actually be subscribing to the base class event."... – Zohar Peled Jul 24 '18 at 18:54

1 Answers1

3

In c#, events should not be declared virtual. Instead, when you declare an event in your class, best practice is to also provide a protected virtual method that raise said event - that's the method OnCollectionChanged in the code you provided in the question. (btw, please note that this method will throw an exception if no one is listening to this event - you should make sure it's not null - so change CollectionChanged.Invoke(this, e); to CollectionChanged?.Invoke(this, e);).

The reason for this is that the c# compiler does not handle virtual events correctly, as documented in How to: Raise Base Class Events in Derived Classes (C# Programming Guide) (You can thank @itsme86 for the link in the comments to the question):

Note

Do not declare virtual events in a base class and override them in a derived class. The C# compiler does not handle these correctly and it is unpredictable whether a subscriber to the derived event will actually be subscribing to the base class event.

An event can only be raised from inside the class declaring it, that's why you need the protected virtual method to raise it from the derived class - also documented in the same page:

When you create a class that can be used as a base class for other classes, you should consider the fact that events are a special type of delegate that can only be invoked from within the class that declared them. Derived classes cannot directly invoke events that are declared within the base class.

The correct solution would be to not override the event, but only the method that raise it - that's when you want to change the conditions on which the event will be raised, and you raise it by calling base.OnCollectionChanged(e);.

A correct implementation of your code would look like this:

public class ObservableCollection<T>
{
    // Note: the event is no longer virtual
    public event NotifyCollectionChangedEventHandler CollectionChanged;

    protected virtual void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        CollectionChanged?.Invoke(this, e);
    }
}

public class DerivedObservableCollection<T> : ObservableCollection<T>
{
    private bool _SuppressNotification;

    // This line is removed:
    // public override event NotifyCollectionChangedEventHandler CollectionChanged;

    protected virtual void OnCollectionChangedMultiItem(
        NotifyCollectionChangedEventArgs e)
    {
        NotifyCollectionChangedEventHandler handlers = this.CollectionChanged;
        if (handlers != null)
        {
            foreach (NotifyCollectionChangedEventHandler handler in 
                handlers.GetInvocationList())
            {
                if (handler.Target is CollectionView)
                    ((CollectionView)handler.Target).Refresh();
                else
                    handler(this, e);
            }
        }
    }

    protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
    {
        if (!_SuppressNotification)
        {
            // This is where you raise the event
            base.OnCollectionChanged(e);
            // the next two lines are also removed, since you've already raised the event
            // and you can't raise it directly from the derived class anyway
            
            //if (CollectionChanged != null)
            //    CollectionChanged.Invoke(this, e);
        }
    }
}
Community
  • 1
  • 1
Zohar Peled
  • 79,642
  • 10
  • 69
  • 121
  • Very strange, because MSDN [https://msdn.microsoft.com/en-us/library/ms653375(v=vs.110).aspx] stated System.Collections.ObjectModel.ObservableCollection.CollectionChanged is virtual. – Кое Кто Jul 26 '18 at 16:17