Skip to content

Commit

Permalink
Fixed OrderBy for collection changes
Browse files Browse the repository at this point in the history
  • Loading branch information
YohDeadfall committed Jun 5, 2024
1 parent 4cd9574 commit 253ed1b
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 10 deletions.
28 changes: 21 additions & 7 deletions src/Kinetic/Linq/ObservableView.OrderBy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,28 @@ public void OnNext(ValueTuple<TItem> value)
{
Debug.Assert(typeof(TItem) == typeof(DynamicItem));

var item = value.Item1;
// Old index search is based on reference equality,
// while the new index is key comparison based.
ref var item = ref value.Item1;
// The index search uses two separate BinarySearch calls
// to exclude the current item which already has a new key.
var oldIndex = _items.IndexOf(item);
var newIndex = _items.BinarySearch(item, _keyComparer.ItemComparer);
var newIndex = oldIndex;

if (newIndex < 0)
newIndex = ~newIndex;
if (oldIndex > 0)
{
newIndex = _items.BinarySearch(index: 0, count: oldIndex, item, _keyComparer.ItemComparer);

if (newIndex < 0)
newIndex = ~newIndex;
}

if (newIndex == oldIndex &&
newIndex < _items.Count - 1)
{
newIndex = _items.BinarySearch(newIndex + 1, _items.Count - newIndex - 1, item, _keyComparer.ItemComparer);

if (newIndex < 0)
newIndex = ~newIndex;
}

if (oldIndex != newIndex)
{
Expand Down Expand Up @@ -375,7 +389,7 @@ private int UpdateIndexes(int oldIndex, int newIndex)
{
foreach (ref var current in indexes)
{
if (current > newIndex && current < oldIndex)
if (current >= newIndex && current < oldIndex)
current += 1;
}

Expand Down
6 changes: 3 additions & 3 deletions test/Kinetic.Tests/ObservableViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,10 @@ public void OrderByDynamic()
Assert.Equal(new[] { itemA, itemB, itemC, itemD }, view);

itemA.Number.Set(5);
itemB.Number.Set(4);
itemC.Number.Set(0);

Assert.Equal(new[] { itemA, itemC, itemD, itemB }, list);
Assert.Equal(new[] { itemC, itemD, itemB, itemA }, view);
Assert.Equal(new[] { itemC, itemB, itemD, itemA }, view);

list.Remove(itemA);
list.Remove(itemD);
Expand Down Expand Up @@ -111,7 +111,7 @@ public void OrderByDynamic()
list.Add(itemB);

Assert.Equal(new[] { itemA, itemC, itemD, itemB }, list);
Assert.Equal(new[] { itemC, itemD, itemB, itemA }, view);
Assert.Equal(new[] { itemC, itemB, itemD, itemA }, view);
}

[Fact]
Expand Down

0 comments on commit 253ed1b

Please sign in to comment.