Return to Answer

• 1

• 50.9k
• 5
• 83
• 177

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: http://programmers.stackexchange.com/q/7823/100919