# Keeping an observable collection up to date

I'm using an ObservableCollection in a WPF app. The source of the data doesn't provide changes. It provides snapshots instead. The method I wrote to update the ObservableCollection works but it seems inelegant. Is there a better way to do this?

    public static void UpdateMarketOrderList(
ObservableCollection<MarketOrder> oldList,
List<MarketOrder> newList)
{
int oi = 0; // old list iterator
int ni = 0; // new list iterator

while (ni < newList.Count)
{
if (oi == oldList.Count)
{
oldList.Insert(oi, newList[ni]);
}
else if (oldList[oi].Price == newList[ni].Price)
{
if (oldList[oi].Quantity != newList[ni].Quantity)
{
oldList[oi].Quantity = newList[ni].Quantity;
}
++oi;
++ni;
}
else if (oldList[oi].Price < newList[ni].Price)
{
oldList.RemoveAt(oi);
}
else if (oldList[oi].Price > newList[ni].Price)
{
oldList.Insert(oi, newList[ni]);
}
}

while (oldList.Count > ni)
{
oldList.RemoveAt(ni);
}
}


If it matters, the definition MarketOrder can be considered to be this:

public class MarketOrder
{
public decimal Price { get; set; }
public decimal Quantity { get; set; }
}

• Is it assumed that lists are sorted by Price and there are no duplicates per Price? Jun 30, 2011 at 19:33
• Snowbear: Yes. I do an .OrderBy(x => x.Price).ToList() before calling. I should probably do that in the method itself. Jun 30, 2011 at 19:50

## 1 Answer

After studying your code snippet for a bit here's what I came up with.

I wouldn't call them iterators. They're really acting more as indexing operators that doubles as a loop counter. Furthermore, they're always incremented in sync and they have the exact same initial value.

int oi = 0; // old list iterator
int ni = 0; // new list iterator


So I would replace with simply:

int i = 0;


The second loop for trimming the extra elements:

while (oldList.Count > ni)
{
oldList.RemoveAt(ni);
}


The post-condition for i after the first loop has to be equal to newList.Count otherwise we would still be looping the first while. Thusly, changing the condition to this is clearer:

while (oldList.Count > newList.Count)


After tracing your UpdateMarketOrderList method, the discernible end effect, from what I can tell, just seems to be a replacement of the oldList with the newList. If that's really the desired end consequence of running this function, why not simplify it, succinctly to this instead?

public static void UpdateMarketOrderList( ObservableCollection<MarketOrder> oldList,
List<MarketOrder> newList )
{
oldList.Clear();

for(int i = 0; i < newList.Count; ++i)
{
oldList.Add(newList[i]);
}
}


Then again you didn't specify in your question what the pre/post-conditions are for UpdateMarketOrderList, which is something that does matter.

After reading OP's comment that fired change events are important, I would refactor the main loop like this:

    while (i < newList.Count)
{
if (i == oldList.Count ||
oldList[i].Price > newList[i].Price)
{
oldList.Insert(i, newList[ni]);
}
else if (oldList[i].Price < newList[i].Price)
{
oldList.RemoveAt(i);
continue;
}
else if (oldList[i].Quantity != newList[i].Quantity)
// && oldList[i].Price == newList[i].Price
{
Debug.Assert(oldList[i].Price == newList[i].Price);
oldList[i].Quantity = newList[i].Quantity;
}

++i;
}


With this refactor you decrease the indent level and tighten the loop a bit while preserving existing behavior. The key thing to realize is that RemoveAt doesn't increment the loop counter. Your previous code did that in a slightly less direct manner.

My gut feeling is that this could be refactored further still but without a good idea of the expected side-effects and other requirements for this method, it will be difficult to refactor this safely.

• Clearing and then adding elements isn't acceptable because the change information is lost. Notice that it is an ObservableCollection and changes fire events. That said, the first half of your answer is gold. Thanks! Jul 2, 2011 at 8:44
• @Jere I remember reading about the fire events in msdn but didn't realize at the time it was important for your scenario. I've edited my answer with more info. Jul 13, 2011 at 1:38