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

Splitting two ObservableCollections lightning fast

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

Problem

I'm working on wiring up a bindable Map for Xamarin Forms, and I need a fast way of dividing up the ObservableCollections (collection of map markers) as quickly as possible. Collections are a weak area for me, and I find myself doing too many foreach's when they're not necessary.

Here's what I have now.

private static void MarkerSourceCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
{
    var markers = ((IEnumerable) sender).ToList();

    var comparer = new MapMarkerComparer();
    var offScreen = new List();
    var markersToAdd = markers.Where(m => _map.MapBounds.Contains(m.CenterPoint), out offScreen)
        .Except(_oldMarkers, comparer);

    var markersToRemove = _oldMarkers.Except(markers, comparer).Union(offScreen);
    _map.HandleMarkersChanged(markers, new MapMarkersChangedEventArgs(markersToAdd, markersToRemove));
    _oldMarkers = markers;
}


With the comparer

internal class MapMarkerComparer : IEqualityComparer
{
    public bool Equals(XfxMarker x, XfxMarker y)
    {
        if (ReferenceEquals(x, y)) return true;
        if (ReferenceEquals(x, null) || ReferenceEquals(y, null)) return false;
        var result = x.CenterPoint.Latitude.ToString() == y.CenterPoint.Latitude.ToString()
                     && x.CenterPoint.Longitude.ToString() == y.CenterPoint.Longitude.ToString();
        return result;
    }

    public int GetHashCode(XfxMarker obj)
    {
        unchecked
        {
            var hash = 0;
            if (obj.CenterPoint == null) return hash;
            hash = hash + 17;
            hash = hash*23 + obj.CenterPoint.Longitude.GetHashCode();
            hash = hash*23 + obj.CenterPoint.Latitude.GetHashCode();
            return hash;
        }
    }
}


And my custom Where extension.

```
public static IEnumerable Where(this IEnumerable input, Func predicate, out List remainder)
{
var leftover = new List();
var output = new List();
foreach (var item in input)

Solution

NotifyCollectionChangedEventArgs

You receive a NotifyCollectionChangedEventArgs, wich holds interesting data about the items that got added NewItems and removed OldItems from the collection. There is also an Action that defines how the collection got modified. Your changed handler is implemented as a NotifyCollectionChangedAction.Reset handler. It doesn't take into account the information from the event e.

var markers = ((IEnumerable) sender).ToList();


This means, you are checking all elements against the previously cached elements by the handler container class and manually verify which ones are added and which ones removed. Let's say filter is m => _map.MapBounds.Contains(m.CenterPoint):

var markersToAdd = markers
    .Where(m => filter(m), out offScreen).Except(_oldMarkers,comparer);


and

var markersToRemove = _oldMarkers.Except(markers, comparer).Union(offScreen);


However, in most cases I presume the action to be Add or Remove. If this is the case, you already have the new and/or old elements. For instance, on Add:

var markersAddedInSource = e.NewItems.OfType();
var markersToAdd = markersAddedInSource.Where(filter);
var markersToRemove = markersAddedInSource.Except(markersToAdd);


Or using your own extension method:

var markersAddedInSource = e.NewItems.OfType();
var markersToAdd = markersAddedInSource.Where(filter, out var markersToRemove);


This saves you a couple of iterations on the source collection and allows you to only iterate on the changed elements.

MapMarkerComparer

I don't know the type of Latitude and Longitude, but consider checking equality on their source type rather than their ToString outcome.

x.CenterPoint.Latitude.ToString() == y.CenterPoint.Latitude.ToString()


In case you have issues with rounding or truncation (likely with float or double) you could round the values in the check.

x.CenterPoint.Latitude == y.CenterPoint.Latitude


Or you could use rounding:

Math.Round(x.CenterPoint.Latitude, precision) 
    == Math.Round(y.CenterPoint.Latitude, precision)


Or you could guard against a tolerance threshold:

Math.Abs(x.CenterPoint.Latitude - y.CenterPoint.Latitude) < tolerance


IEnumerable Where

Consider changing the method signature to

public static ILookup Where(this IEnumerable input, Func predicate);


Or returning a Dictionary:

public static IDictionary> Where(
    this IEnumerable input, Func predicate);


So you don't need to worry about a return value and an out-parameter. This pattern isn't used that much anymore and is mostly used to Try* operations.

You can get the items that matched the predicate:

var results = markers.Where(filter);
var matched = results[true];
var unmatched = results[false];


Considering renaming Where to avoid confusion with LINQ's Where method.

Code Snippets

var markers = ((IEnumerable<XfxMarker>) sender).ToList();
var markersToAdd = markers
    .Where(m => filter(m), out offScreen).Except(_oldMarkers,comparer);
var markersToRemove = _oldMarkers.Except(markers, comparer).Union(offScreen);
var markersAddedInSource = e.NewItems.OfType<XfxMarker>();
var markersToAdd = markersAddedInSource.Where(filter);
var markersToRemove = markersAddedInSource.Except(markersToAdd);
var markersAddedInSource = e.NewItems.OfType<XfxMarker>();
var markersToAdd = markersAddedInSource.Where(filter, out var markersToRemove);

Context

StackExchange Code Review Q#119028, answer score: 4

Revisions (0)

No revisions yet.