SortedBindingList Delete Bug

xal posted on Monday, November 20, 2006

Hey everyone, I've bumped into an issue with the sorted binding list. I've tested it in 2.1 and 2.1.1 and I get the same results.

If you want to avoid the explanation, I've attached a very simple test case of which only the Form_Load code is relevant (6 lines of code).

The problem goes like this:

1. Create a new sorted binding list that points to an editable collection.
2. Add a new item to the list (whether you do it through the sorted binding list or the actual list is irrelevant).
3. Sort the list in descending order.
4. Remove the newly added item from the sorted list using it's index by using RemoveAt().

Passing the actual object to remove() works correctly. The problem is that the new object's index is stored as if it were the count of the list instead of the count -1. I'll see if I can find where the problem is.


Andrés

xal replied on Monday, November 20, 2006

Ok, I've found the problem.

Inside RemoveAt() there's this line:
mSortIndex.RemoveAt(index)

It should be changed for this:
            If mSortOrder = ListSortDirection.Ascending Then
                mSortIndex.RemoveAt(index)
            Else
                mSortIndex.RemoveAt(mSortIndex.Count - 1 - index)
            End If

Rocky, can you take a look at this?

Andrés

RockfordLhotka replied on Tuesday, November 21, 2006

I have fixed it in cvs, thanks for finding this.

oshokodk replied on Thursday, December 07, 2006

I found one more problem with RemoveAt(int index) method.

 

If any ListChange event is generated during item deletion then by the time _list.RemoveAt(baseIndex) returns, DoSort() has been executed and _sortIndex list is up to date. Any further adjustment to this list make it invalid. When grid refreshes itself, exception occurs. In a hurry I fixed this problem by changing two functions:

 

public void RemoveAt(int index)

{

      if (_sorted)

      {

        _initiatedLocally = true;

        int baseIndex = OriginalIndex(index);

        // remove the item from the source list

        _list.RemoveAt(baseIndex);

 

//if equal, DoSort has happened in reponse to event fired in //_list.RemoveAt. Aadjust sorted index only if needed.

        if (_list.Count != _sortIndex.Count)   

        {  

            // delete the corresponding value in the sort index

            if (_sortOrder == ListSortDirection.Ascending)

                _sortIndex.RemoveAt(index);

            else

                _sortIndex.RemoveAt(_sortIndex.Count - 1 - index);

            // now fix up all index pointers in the sort index

            foreach (ListItem item in _sortIndex)

                if (item.BaseIndex > baseIndex)

                    item.BaseIndex -= 1;

        }

        OnListChanged(new ListChangedEventArgs(

          ListChangedType.ItemDeleted, index));

        _initiatedLocally = false;

      }

      else

        _list.RemoveAt(index);

}

 

private void SourceChanged(

{

...

          default:

            // for anything other than add, delete or change

            // just re-sort the list

            if (!_initiatedLocally)

                DoSort();

            break;

}

RockfordLhotka replied on Wednesday, January 10, 2007

---

From: oshokodk [mailto:cslanet@lhotka.net]
Sent: Thursday, December 07, 2006 4:27 PM
To: rocky@lhotka.net
Subject: Re: [CSLA .NET] SortedBindingList Delete Bug

I found one more problem with RemoveAt(int index) method.

If any ListChange event is generated during item deletion then by the time _list.RemoveAt(baseIndex) returns, DoSort() has been executed and _sortIndex list is up to date. Any further adjustment to this list make it invalid. When grid refreshes itself, exception occurs. In a hurry I fixed this problem by changing two functions:

public void RemoveAt(int index)

{

      if (_sorted)

      {

        _initiatedLocally = true;

        int baseIndex = OriginalIndex(index);

        // remove the item from the source list

        _list.RemoveAt(baseIndex);

//if equal, DoSort has happened in reponse to event fired in //_list.RemoveAt. Aadjust sorted index only if needed.

        if (_list.Count != _sortIndex.Count)   

        {  

            // delete the corresponding value in the sort index

            if (_sortOrder == ListSortDirection.Ascending)

                _sortIndex.RemoveAt(index);

            else

                _sortIndex.RemoveAt(_sortIndex.Count - 1 - index);

            // now fix up all index pointers in the sort index

            foreach (ListItem item in _sortIndex)

                if (item.BaseIndex > baseIndex)

                    item.BaseIndex -= 1;

        }

        OnListChanged(new ListChangedEventArgs(

          ListChangedType.ItemDeleted, index));

        _initiatedLocally = false;

      }

      else

        _list.RemoveAt(index);

}

private void SourceChanged(

{

...

          default:

            // for anything other than add, delete or change

            // just re-sort the list

            if (!_initiatedLocally)

                DoSort();

            break;

}