HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Mapping ReadOnlyObservableCollection to another collection

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mappingcollectionanotherreadonlyobservablecollection

Problem

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
{
///
/// 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.
///
/// The type of the instances in the inner collection
/// The type of the instances that are exposed to users of this class
public class TransformedCollection
: INotifyCollectionChanged, IReadOnlyCollection
{
public event NotifyCollectionChangedEventHandler CollectionChanged;

private IReadOnlyCollection innerCollection;
private List transformedValues = new List();
private Func transformationFunction;

///
/// Creates a new transformed collection to view another collection.
///
/// The collection containing the instances to transform. Must not be null.
/// The function to convert an inner collection instance. Must not be null.
/// This delegate must not throw any exceptions.
public TransformedCollection(IReadOnlyCollection innerCollection, Func

Solution

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.

Code Snippets

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();
        }
    }

Context

StackExchange Code Review Q#21317, answer score: 4

Revisions (0)

No revisions yet.