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

Keeping an observable collection up to date

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

Problem

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 oldList, 
                          List newList)
    {
        int oi = 0; // old list iterator
        int ni = 0; // new list iterator

        while (ni  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; }
}

Solution

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 oldList, 
                                          List 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[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.

Code Snippets

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

    for(int i = 0; i < newList.Count; ++i)
    {
        oldList.Add(newList[i]);
    }
}
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;
    }

Context

StackExchange Code Review Q#3231, answer score: 3

Revisions (0)

No revisions yet.