What’s Wrong With This Code #24

Wednesday, February 10, 2010

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.


Comments
gravatar Bill Wednesday, February 10, 2010
Bowlers is returning a sorted copy of _bowlers, not doing the sort operation on _bowlers.
Knaģis Thursday, February 11, 2010
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 zihotki Thursday, February 11, 2010
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 Pete Thursday, February 11, 2010
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 :)
OmariO Thursday, February 11, 2010
OrderBy returns a new collection that is not observable
gravatar Wes Friday, February 12, 2010
A CollectionViewSource is a good fix, can even be done with MVVM.
gravatar Paul Stovell Saturday, February 20, 2010
This is what projects like Bindable LINQ (http://bindablelinq.codeplex.com) were created for.

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

The result implements INotifyCollectionChanged.
gravatar Lloyd Dupont Wednesday, February 2, 2011
Bindable LINQ seem awesome! :)

This is the official solution
msdn.microsoft.com/...
Comments are now closed.
by K. Scott Allen K.Scott Allen
My Pluralsight Courses
The Podcast!