What’s Wrong With This Code #24

Sometimes the simplest solution introduces a bug.

Let’s say there is a ListBox in WPF (or Silverlight) displaying bowlers* and their bowling scores:

 <ListBox ItemsSource="{Binding Bowlers}" >
     <ListBox.ItemTemplate>
         <DataTemplate>
             <StackPanel Orientation="Horizontal">
                 <TextBlock Text="{Binding Name}"/>
                 <TextBlock Text="{Binding Score}"/>
             </StackPanel>                             
         </DataTemplate>
     </ListBox.ItemTemplate>
 </ListBox>

Behind the scenes, a view model includes a Bowlers property for the binding to work.

public IEnumerable<Bowler> Bowlers 
{ 
    get
    {
        return _bowlers;
    }
    set
    {
        if (value != _bowlers)
        {
            _bowlers = value.ToObservable();
            // ... Raise property changed event
        }
    }
}

The property uses a ToObservable extension method that works in either Silverlight or WPF.

public static class ObservableExtensions
{
    public static ObservableCollection<T> ToObservable<T>(this IEnumerable<T> items)
    {
        var collection = new ObservableCollection<T>();
        foreach(var item in items)
        {
            collection.Add(item);
        }
        return collection;
    }
}

So far everything is working. Bowlers appear on the screen, and as new bowlers get added to the collection …

_bowlers.Add(newBowler);

… they magically appear in the display.

The Change

Now someone wants the bowlers list sorted with the highest score appearing at the top of the list. Sounds like a simple change to the Bowlers property:

public IEnumerable<Bowler> Bowlers 
{ 
    get
    {
        return _bowlers.OrderByDescending(bowler => bowler.Score);
    }
    set
    {
        // same as before ...
    }
}

At first glace, the new code works and the bowlers appear in sorted order, but something is wrong! What will break?

Hint:

Bowlers added with _bowlers.Add(…) never appear in the display. Why is that?

 

* Of the ten pin variety - not the spinning kind.

Print | posted @ Wednesday, February 10, 2010 9:12 PM

Comments on this entry:

Gravatar # re: What’s Wrong With This Code #24
by Bill at 2/10/2010 11:45 PM

Bowlers is returning a sorted copy of _bowlers, not doing the sort operation on _bowlers.
  
Gravatar # re: What’s Wrong With This Code #24
by Knaģis at 2/11/2010 3:12 AM

the binding is done to a IOrderEnumerator (or whatever the actual name is) not on ObservableCollection anymore because that is what the Bowlers now return. So the UI engine cannot detect that the collection has been changed.
  
Gravatar # re: What’s Wrong With This Code #24
by zihotki at 2/11/2010 4:08 AM

I second @Knagis. OrderByDescending returns a new IEnumerable object that is not implements IObservable so it can't be casted to IObservable. You have to implement some custom collection/model that will implement IObservable and order elements in it.
  
Gravatar # re: What’s Wrong With This Code #24
by Pete at 2/11/2010 12:46 PM

The problem is you used a stackpanel for your display, so each row in your listbox is going to have a different left position for the score ;)

Oh, and you lost the observable collection when you returned the sort.

Neat series :)
  
Gravatar # re: What’s Wrong With This Code #24
by OmariO at 2/11/2010 4:08 PM

OrderBy returns a new collection that is not observable
  
Gravatar # re: What’s Wrong With This Code #24
by Wes at 2/12/2010 1:47 AM

A CollectionViewSource is a good fix, can even be done with MVVM.
  
Gravatar # re: What’s Wrong With This Code #24
by Paul Stovell at 2/20/2010 6:10 AM

This is what projects like Bindable LINQ (http://bindablelinq.codeplex.com) were created for.

return _bowlers.AsBindable().OrderByDescending(...)

The result implements INotifyCollectionChanged.
  
Gravatar # re: What’s Wrong With This Code #24
by Lloyd Dupont at 2/2/2011 10:18 PM

Bindable LINQ seem awesome! :)

This is the official solution
msdn.microsoft.com/...
  
Comments have been closed on this topic.
Scott Allen
Posts - 869
Comments - 4493
Stories - 14