-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Closed
Labels
Description
The following test fails:
[Fact]
public void derived_collection_ordering_bug()
{
var collection = new ReactiveCollection<int>();
var orderedCollection = collection.CreateDerivedCollection(x => x, null, (x, y) => x.CompareTo(y));
collection.Add(1);
collection.Add(2);
Assert.Equal(2, orderedCollection.Count);
Assert.Equal(1, orderedCollection[0]);
Assert.Equal(2, orderedCollection[1]);
}This is due to the following lines in ReactiveCollection.positionForNewItem:
if (list.Count == 1) {
return orderer(list[0], item) >= 0 ? 1 : 0;
}I believe it should be:
if (list.Count == 1) {
return orderer(list[0], item) >= 0 ? 0 : 1;
}which allows the test to pass and upholds the same insertion behavior when items are equal that is exhibited later in the code (ie. new item is inserted before existing item if they are equal).
Alternatively, you could just remove the list.Count == 1 path altogether - seems it's optimizing for the uncommon case to me.
Again, apologies for not being able to submit a pull request - corporate proxy forbids it.