6
\$\begingroup\$

I'm writing a WPF application and I have several ReadOnlyObservableCollection fields in my models.

Suppose that I wanted to create a FooViewModel instance for each FooModel instance. FooModel has an observable collection of BarModel, and FooViewModel should contain a (read-only) observable collection of BarViewModel.

I created an utility class to simplify this task:

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.Diagnostics.Contracts;
using System.Linq;

namespace NamespaceNameGoesHere
{
    /// <summary>
    /// Creates a view of another ("inner") read-only collection, using a function to map each instance in the source collection to another instance that is viewed by users of this class.
    /// If the inner collection uses the CollectionChanged event correctly, then the two are kept in sync.
    /// Note that when there are changes to the inner collection, some transformed instances might be recreated. Any attempt to prevent this should 
    /// This class is NOT thread-safe.
    /// </summary>
    /// <typeparam name="InnerType">The type of the instances in the inner collection</typeparam>
    /// <typeparam name="TargetType">The type of the instances that are exposed to users of this class</typeparam>
    public class TransformedCollection<InnerType, TargetType>
        : INotifyCollectionChanged, IReadOnlyCollection<TargetType>
    {
        public event NotifyCollectionChangedEventHandler CollectionChanged;

        private IReadOnlyCollection<InnerType> innerCollection;
        private List<TargetType> transformedValues = new List<TargetType>();
        private Func<InnerType, TargetType> transformationFunction;

        /// <summary>
        /// Creates a new transformed collection to view another collection.
        /// </summary>
        /// <param name="innerCollection">The collection containing the instances to transform. Must not be null.</param>
        /// <param name="transformationFunction">The function to convert an inner collection instance. Must not be null.
        /// This delegate must not throw any exceptions.</param>
        public TransformedCollection(IReadOnlyCollection<InnerType> innerCollection, Func<InnerType, TargetType> transformationFunction)
        {
            Contract.Requires<ArgumentNullException>(innerCollection != null, "innerCollection must not be null");
            Contract.Requires<ArgumentNullException>(transformationFunction != null, "transformationFunction must not be null");

            this.innerCollection = innerCollection;
            this.transformationFunction = transformationFunction;

            CreateCollection();

            //Ensure that when the inner collection is changed, the changes propagate to this one.
            ((INotifyCollectionChanged)innerCollection).CollectionChanged += InnerCollectionChanged;
        }

        /// <summary>
        /// Transforms and adds every element in the inner list
        /// </summary>
        private void CreateCollection()
        {
            foreach (var element in CreateTransformedElements())
            {
                transformedValues.Add(element);
            }
        }

        /// <summary>
        /// Transforms all elements in the inner list.
        /// </summary>
        /// <returns>An enumerable containing all the transformed elements.</returns>
        private IEnumerable<TargetType> CreateTransformedElements()
        {
            return innerCollection
                .Select(inner => transformationFunction(inner));
        }

        private void InnerCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
        {
            List<TargetType> newItems = null;
            List<TargetType> oldItems = null;

            switch (e.Action)
            {
                case NotifyCollectionChangedAction.Add:
                    newItems = new List<TargetType>();

                    for (int i = 0; i < e.NewItems.Count; ++i)
                    {
                        //Transform all newly added items and add them to temporary "newItems" list.

                        int index = e.NewStartingIndex + i;
                        InnerType innerElement = (InnerType)e.NewItems[i];

                        TargetType transformedElement = transformationFunction(innerElement);

                        newItems.Add(transformedElement);
                    }

                    transformedValues.InsertRange(e.NewStartingIndex, newItems);

                    RaiseCollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, newItems, e.NewStartingIndex));

                    break;
                case NotifyCollectionChangedAction.Remove:
                    oldItems = new List<TargetType>();

                    for (int i = 0; i < e.OldItems.Count; ++i)
                    {
                        int index = e.OldStartingIndex + i;

                        oldItems.Add(transformedValues[index]);
                    }

                    transformedValues.RemoveRange(e.OldStartingIndex, e.OldItems.Count);

                    RaiseCollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, oldItems, e.OldStartingIndex));

                    break;
                case NotifyCollectionChangedAction.Replace:
                    newItems = new List<TargetType>();
                    oldItems = new List<TargetType>();

                    for (int i = 0; i < e.NewItems.Count; ++i)
                    {
                        int index = e.NewStartingIndex + i;
                        InnerType innerElement = (InnerType)e.NewItems[i];

                        TargetType transformedElement = transformationFunction(innerElement);

                        newItems.Add(transformedElement);
                        oldItems.Add(transformedValues[index]);

                        transformedValues[index] = transformedElement;
                    }

                    RaiseCollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, newItems, oldItems, e.NewStartingIndex));

                    break;
                case NotifyCollectionChangedAction.Reset:
                    transformedValues.Clear();

                    CreateCollection();

                    RaiseCollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));

                    break;
                case NotifyCollectionChangedAction.Move:
                    newItems = new List<TargetType>();

                    if (e.NewItems.Count != 1)
                    {
                        throw new NotImplementedException("No idea how this is supposed to work");
                    }

                    //The names "at old index"/"at new index" refer to the positions *before* the move method was called.
                    TargetType valueAtOldIndex = transformedValues[e.OldStartingIndex];
                    TargetType valueAtNewIndex = transformedValues[e.NewStartingIndex];

                    //Swap positions
                    transformedValues[e.NewStartingIndex] = valueAtOldIndex;
                    transformedValues[e.OldStartingIndex] = valueAtNewIndex;

                    //The value at the old index *before* the move method was called is the value that is at the new index *after* it.
                    //We add that value to the list
                    newItems.Add(valueAtOldIndex);

                    RaiseCollectionChanged(this, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, newItems, e.NewStartingIndex, e.OldStartingIndex));

                    break;
                default:
                    throw new InvalidOperationException();
            }
        }

        private void RaiseCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
        {
            if (CollectionChanged != null)
            {
                CollectionChanged(sender, e);
            }
        }

        public IEnumerator<TargetType> GetEnumerator()
        {
            return transformedValues.GetEnumerator();
        }

        IEnumerator IEnumerable.GetEnumerator()
        {
            return GetEnumerator();
        }

        public int Count
        {
            get
            {
                return transformedValues.Count;
            }
        }
    }
}

NamespaceNameGoesHere is not the real name of the namespace.

I have created the following xUnit tests which all pass:

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Collections.ObjectModel;
using Xunit;
using System.Collections.Specialized;

namespace NamespaceNameGoesHere.Test
{
    public class TransformedCollectionTest
    {
        private int Double(int x)
        {
            return x * 2;
        }

        [Fact]
        public void NullArgumentsTest()
        {
            var emptySourceCollection = new ObservableCollection<int>();
            var emptyCollection = new ReadOnlyObservableCollection<int>(emptySourceCollection);

            Assert.Throws<ArgumentNullException>(() => new TransformedCollection<int, int>(null, Double));
            Assert.Throws<ArgumentNullException>(() => new TransformedCollection<int, int>(emptyCollection, null));
            Assert.Throws<ArgumentNullException>(() => new TransformedCollection<int, int>(null, null));
        }

        [Fact]
        public void InitialStateTest()
        {
            var emptyCollection = new ObservableCollection<int>();
            var emptyReadOnlyCollection = new ReadOnlyObservableCollection<int>(emptyCollection);

            var emptyTransformCollection = new TransformedCollection<int, int>(emptyReadOnlyCollection, Double);

            //Collection must be empty
            Assert.Equal(0, emptyTransformCollection.Count);

            //Additionally, the enumerator (from IEnumerable<T>) will give no result
            Assert.Equal(false, emptyTransformCollection.GetEnumerator().MoveNext());

            //Check the same with the non-generic enumerator
            Assert.Equal(false, ((IEnumerable)emptyTransformCollection).GetEnumerator().MoveNext());

            var simpleCollection = new ObservableCollection<int>();
            simpleCollection.Add(1);
            simpleCollection.Add(2);
            simpleCollection.Add(3);

            var simpleReadOnlyCollection = new ReadOnlyObservableCollection<int>(simpleCollection);

            var simpleTransformCollection = new TransformedCollection<int, int>(simpleReadOnlyCollection, Double);

            Assert.Equal(3, simpleTransformCollection.Count);

            //We expect to get [2, 4, 6]
            int i = 2;
            foreach (int value in simpleTransformCollection)
            {
                Assert.Equal(i, value);

                i += 2;
            }
        }

        [Fact]
        public void AddElementTest()
        {
            var startIndices = new List<int>();
            var newItems = new List<IList>();

            var baseCollection = new ObservableCollection<int>();
            var readOnlyCollection = new ReadOnlyObservableCollection<int>(baseCollection);
            var transformedCollection = new TransformedCollection<int, int>(readOnlyCollection, Double);

            transformedCollection.CollectionChanged += (sender, args) =>
            {
                Assert.Same(transformedCollection, sender);

                Assert.Equal(NotifyCollectionChangedAction.Add, args.Action);

                //Keep track of the events
                startIndices.Add(args.NewStartingIndex);
                newItems.Add(args.NewItems);
            };

            Assert.Equal(0, startIndices.Count);

            baseCollection.Add(1);

            Assert.Equal(1, transformedCollection.Count);
            Assert.Equal(2, transformedCollection.First());

            //Check the events
            Assert.Equal(1, startIndices.Count);
            Assert.Equal(0, startIndices.First());
            Assert.Equal(1, newItems.First().Count);
            Assert.Equal(2, newItems.First()[0]);

            baseCollection.Add(2);

            Assert.Equal(2, transformedCollection.Count);
            Assert.Equal(2, transformedCollection.First());
            Assert.Equal(4, transformedCollection.ElementAt(1));

            //Check the events
            Assert.Equal(2, startIndices.Count);
            Assert.Equal(1, startIndices.ElementAt(1));
            Assert.Equal(1, newItems.ElementAt(1).Count);
            Assert.Equal(4, newItems.ElementAt(1)[0]);
        }

        [Fact]
        public void RemoveElementTest()
        {
            var startIndices = new List<int>();
            var oldItems = new List<IList>();

            var baseCollection = new ObservableCollection<int>();
            baseCollection.Add(1);
            baseCollection.Add(2);
            baseCollection.Add(3);

            var readOnlyCollection = new ReadOnlyObservableCollection<int>(baseCollection);
            var transformedCollection = new TransformedCollection<int, int>(readOnlyCollection, Double);

            transformedCollection.CollectionChanged += (sender, args) =>
            {
                Assert.Same(transformedCollection, sender);

                Assert.Equal(NotifyCollectionChangedAction.Remove, args.Action);

                //Keep track of the events
                startIndices.Add(args.OldStartingIndex);
                oldItems.Add(args.OldItems);
            };

            Assert.Equal(3, transformedCollection.Count);
            Assert.Equal(0, startIndices.Count);

            Assert.Equal(2, transformedCollection.First());
            Assert.Equal(4, transformedCollection.ElementAt(1));
            Assert.Equal(6, transformedCollection.ElementAt(2));


            baseCollection.RemoveAt(1);

            Assert.Equal(2, transformedCollection.Count);
            Assert.Equal(2, transformedCollection.First());
            Assert.Equal(6, transformedCollection.ElementAt(1));

            //Check the events
            Assert.Equal(1, startIndices.Count);
            Assert.Equal(1, startIndices[0]);
            Assert.Equal(1, oldItems.First().Count);
            Assert.Equal(4, oldItems.First()[0]);


            baseCollection.Remove(1);

            Assert.Equal(1, transformedCollection.Count);
            Assert.Equal(6, transformedCollection.First());

            //Check the events
            Assert.Equal(2, startIndices.Count);
            Assert.Equal(0, startIndices[1]);
            Assert.Equal(1, oldItems.ElementAt(1).Count);
            Assert.Equal(2, oldItems.ElementAt(1)[0]);


            baseCollection.RemoveAt(0);

            Assert.Equal(0, transformedCollection.Count);

            //Check the events
            Assert.Equal(3, startIndices.Count);
            Assert.Equal(0, startIndices[2]);
            Assert.Equal(1, oldItems.ElementAt(2).Count);
            Assert.Equal(6, oldItems.ElementAt(2)[0]);
        }

        [Fact]
        public void ClearCollectionTest()
        {
            var addStartIndices = new List<int>();
            var actions = new List<NotifyCollectionChangedAction>();

            var baseCollection = new ObservableCollection<int>();
            baseCollection.Add(1);
            baseCollection.Add(2);
            baseCollection.Add(3);

            var readOnlyCollection = new ReadOnlyObservableCollection<int>(baseCollection);
            var transformedCollection = new TransformedCollection<int, int>(readOnlyCollection, Double);

            transformedCollection.CollectionChanged += (sender, args) =>
            {
                Assert.Same(transformedCollection, sender);

                actions.Add(args.Action);

                switch (args.Action)
                {
                    case NotifyCollectionChangedAction.Add:
                        addStartIndices.Add(args.NewStartingIndex);

                        break;
                    case NotifyCollectionChangedAction.Reset:
                        //All we need to do is keep track that it happened, which we already do above
                        //(outside the switch statement)

                        break;
                    default:
                        Assert.True(false, "Unexpected event type: " + args.Action);
                        break;
                }
            };

            Assert.Equal(3, transformedCollection.Count);
            Assert.Equal(0, actions.Count);


            baseCollection.Clear();

            Assert.Equal(0, transformedCollection.Count);

            //Check the events
            Assert.Equal(1, actions.Count);
            Assert.Equal(NotifyCollectionChangedAction.Reset, actions[0]);


            baseCollection.Add(1);

            Assert.Equal(1, transformedCollection.Count);
            Assert.Equal(2, transformedCollection.ElementAt(0));

            //Check the events
            Assert.Equal(2, actions.Count);
            Assert.Equal(NotifyCollectionChangedAction.Add, actions[1]);
            Assert.Equal(1, addStartIndices.Count);
            Assert.Equal(0, addStartIndices[0]);


            baseCollection.Clear();

            Assert.Equal(0, transformedCollection.Count);

            //Check the events
            Assert.Equal(3, actions.Count);
            Assert.Equal(NotifyCollectionChangedAction.Reset, actions[2]);
        }

        [Fact]
        public void SetItemCollectionTest()
        {
            var oldStartIndices = new List<int>();
            var newStartIndices = new List<int>();
            var oldItems = new List<IList>();
            var newItems = new List<IList>();

            var baseCollection = new ObservableCollection<int>();
            baseCollection.Add(1);
            baseCollection.Add(2);
            baseCollection.Add(3);

            var readOnlyCollection = new ReadOnlyObservableCollection<int>(baseCollection);
            var transformedCollection = new TransformedCollection<int, int>(readOnlyCollection, Double);

            transformedCollection.CollectionChanged += (sender, args) =>
            {
                Assert.Same(transformedCollection, sender);

                Assert.Equal(NotifyCollectionChangedAction.Replace, args.Action);

                //Keep track of the events
                oldStartIndices.Add(args.OldStartingIndex);
                newStartIndices.Add(args.NewStartingIndex);
                oldItems.Add(args.OldItems);
                newItems.Add(args.NewItems);
            };

            Assert.Equal(0, oldStartIndices.Count);


            baseCollection[0] = 10;

            Assert.Equal(3, transformedCollection.Count);
            Assert.Equal(20, transformedCollection.ElementAt(0));
            Assert.Equal(4, transformedCollection.ElementAt(1));
            Assert.Equal(6, transformedCollection.ElementAt(2));

            //Check the events
            Assert.Equal(1, oldStartIndices.Count);
            Assert.Equal(0, oldStartIndices[0]);
            Assert.Equal(0, newStartIndices[0]);
            Assert.Equal(1, oldItems[0].Count);
            Assert.Equal(2, oldItems[0][0]);
            Assert.Equal(1, newItems[0].Count);
            Assert.Equal(20, newItems[0][0]);


            baseCollection[2] = 20;

            Assert.Equal(3, transformedCollection.Count);
            Assert.Equal(20, transformedCollection.ElementAt(0));
            Assert.Equal(4, transformedCollection.ElementAt(1));
            Assert.Equal(40, transformedCollection.ElementAt(2));

            //Check the events
            Assert.Equal(2, oldStartIndices.Count);
            Assert.Equal(2, oldStartIndices[1]);
            Assert.Equal(2, newStartIndices[1]);
            Assert.Equal(1, oldItems[1].Count);
            Assert.Equal(6, oldItems[1][0]);
            Assert.Equal(1, newItems[1].Count);
            Assert.Equal(40, newItems[1][0]);
        }

        [Fact]
        public void MoveElementTest()
        {
            var baseCollection = new ObservableCollection<int>();
            baseCollection.Add(1);
            baseCollection.Add(2);
            baseCollection.Add(3);
            baseCollection.Add(4);
            baseCollection.Add(5);

            var readOnlyCollection = new ReadOnlyObservableCollection<int>(baseCollection);

            var transformedCollection = new TransformedCollection<int, int>(readOnlyCollection, Double);

            int events = 0;

            transformedCollection.CollectionChanged += (sender, args) =>
            {
                events++;

                Assert.Equal(1, events);

                Assert.Same(transformedCollection, sender);

                Assert.Equal(1, args.NewItems.Count);
                Assert.Equal(1, args.OldItems.Count);

                Assert.Equal(4, args.NewItems[0]);
                Assert.Equal(4, args.OldItems[0]);

                Assert.Equal(2, args.NewStartingIndex);
                Assert.Equal(1, args.OldStartingIndex);
            };

            baseCollection.Move(1, 2);

            Assert.Equal(5, transformedCollection.Count);
            Assert.Equal(2, transformedCollection.ElementAt(0));
            Assert.Equal(6, transformedCollection.ElementAt(1));
            Assert.Equal(4, transformedCollection.ElementAt(2));
            Assert.Equal(8, transformedCollection.ElementAt(3));
            Assert.Equal(10, transformedCollection.ElementAt(4));
        }
    }
}

I am using only runtime code contract verifications (that is, the static analyzer is turned off).

I am currently satisfied with the current state of the code, but I want to see opinions from other programmers to cover any ground I may have missed.

I am looking for reviews in the following areas:

  • Am I doing this right? That is:
    • Do you find this class useful?
    • Is there a built-in .NET class that already does this (or something very similar)?
  • Am I using IReadOnlyCollection and INotifyCollectionChanged right? Should I implement any other interfaces?
  • Am I doing unit tests right? Are they sufficiently comprehensive?
  • How is the code quality in general? Is it obvious what each part of the code does? Are the comments easy to understand?
  • Is the documentation of the class and public members (that aren't already documented in implemented interfaces) correct? Is it obvious how the class and its methods should be used?
  • Are the variable, method and class names appropriate?
  • Pretty much anything else you might find relevant.
\$\endgroup\$

2 Answers 2

4
+50
\$\begingroup\$

There is way too much going on insider of InnerCollectionChanged. That method is 98 lines of code. This breaks the "Single Screen Principle" by roughly 3 times. What's the Single Screen Principle? It's the idea that any one given method should fit neatly on the screen at once. If you have to scroll to see the rest of the code, there's too much going on and it's very likely that the Single Responsibility Principle is being broken as well.

In fact, InnerCollectionChanged does exactly five different things. It handles the NotifyCollectionChangedAction Add, Remove, Replace, Move and Reset actions. All of the code in each of these cases should be extracted into their own methods. InnerCollectionChanged should look something like this afterward.

    private void InnerCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        switch (e.Action)
        {
            case NotifyCollectionChangedAction.Add:
                HandleAddAction();
                break;
            case NotifyCollectionChangedAction.Remove:
                HandleRemoveAction();
                break;
            case NotifyCollectionChangedAction.Replace:
                HandleReplaceAction();
                break;
            case NotifyCollectionChangedAction.Reset:
                HandleResetAction();
                break;
            case NotifyCollectionChangedAction.Move:
                HandleMoveAction();
                break;
            default:
                throw new InvalidOperationException();
        }
    }

One screen. One job. No scrolling. Immediately understandable.

\$\endgroup\$
2
\$\begingroup\$

Raising Events

The default pattern for rasing events is to create a protected void OnNameOfTheEvent(EventArguments) method (without the sender parameter). Like

protected virtual void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
{
    if (CollectionChanged != null)
    {
        CollectionChanged(this, e);
    }
}   

in this way it can be overridden by a derived class, so the sender this won't be the base object.

InnerCollectionChanged()

This eventhandler is doing a little bit too much and the code is too long (IMHO). You should split the actions to separate methods.

which would result in

private IList<TargetType> GetNewItems(IList items)
{
    IList<TargetType> newItems = new List<TargetType>();
    foreach (object item in items)
    {
        TargetType transformedElement = transformationFunction((InnerType)item);
        newItems.Add(transformedElement);
    }

    return newItems;
}
private IList<TargetType> GetItemsToBeRemoved(IList items, int startIndex)
{
    IList<TargetType> itemsToBeRemoved = new List<TargetType>();
    int endIndex = items.Count + startIndex;
    for (int i = startIndex; i < endIndex; ++i)
    {
        itemsToBeRemoved.Add(transformedValues[i]);
    }
    return itemsToBeRemoved;
}
private IList<TargetType> GetItemsToBeReplaced(IList items, int startIndex)
{
    IList<TargetType> itemsToBeReplaced = new List<TargetType>();
    int endIndex = items.Count + startIndex;
    for (int i = startIndex; i < endIndex; ++i)
    {
        itemsToBeReplaced.Add(transformedValues[i]);
    }
    return itemsToBeReplaced;
}
private void InternalReplace(IList<TargetType> replacementItems, int startIndex)
{
    for (int i = 0; i < replacementItems.Count; ++i)
    {
        transformedValues[i + startIndex] = replacementItems[i];
    }
}
private IList<TargetType> GetToBeReplacedItems(int startIndex, int count)
{
    IList<TargetType> itemsToBeReplaced = new List<TargetType>();
    for (int i = startIndex; i < startIndex + count; i++)
    {
        itemsToBeReplaced.Add(transformedValues[i]);
    }
    return itemsToBeReplaced;
}
private void InternalMove(int sourceIndex, int destinationIndex)
{
    TargetType valueAtOldIndex = transformedValues[sourceIndex];

    transformedValues[sourceIndex] = transformedValues[destinationIndex];
    transformedValues[destinationIndex] = valueAtOldIndex;

}
private void InnerCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
    NotifyCollectionChangedEventArgs eventArgs;

    switch (e.Action)
    {
        case NotifyCollectionChangedAction.Add:
            IList<TargetType> itemsToAdd = GetNewItems(e.NewItems);
            transformedValues.InsertRange(e.NewStartingIndex, itemsToAdd);
            eventArgs = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, itemsToAdd, e.NewStartingIndex);
            break;

        case NotifyCollectionChangedAction.Remove:
            IList<TargetType> itemsToBeRemoved = GetItemsToBeRemoved(e.OldItems, e.OldStartingIndex);
            transformedValues.RemoveRange(e.OldStartingIndex, e.OldItems.Count);
            eventArgs = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, itemsToBeRemoved, e.OldStartingIndex);
            break;

        case NotifyCollectionChangedAction.Replace:

            IList<TargetType> replacementItems = GetNewItems(e.NewItems);
            IList<TargetType> toBeReplacedItems = GetToBeReplacedItems(e.NewStartingIndex, e.NewItems.Count);
            InternalReplace(replacementItems, e.NewStartingIndex);
            eventArgs = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Replace, replacementItems, toBeReplacedItems, e.NewStartingIndex);
            break;

        case NotifyCollectionChangedAction.Reset:
            transformedValues.Clear();
            CreateCollection();
            eventArgs = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset);
            break;

        case NotifyCollectionChangedAction.Move:
            if (e.NewItems.Count != 1)
            {
                throw new NotImplementedException("No idea how this is supposed to work");
            }

            IList<TargetType> movedItems = new List<TargetType>();
            movedItems.Add(transformedValues[e.OldStartingIndex]);
            InternalMove(e.OldStartingIndex, e.NewStartingIndex);
            eventArgs = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, movedItems, e.NewStartingIndex, e.OldStartingIndex);
            break;

        default:
            throw new InvalidOperationException();
    }
    OnCollectionChanged(eventArgs);
}  

and you now have small little methods which you can also test.

Tests

You have a lot of Asserts in your tests. Usually it is seen as bad practice to assert to much. See: https://softwareengineering.stackexchange.com/q/7823/100919

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.