ItemsSourceView.IndexOf API spec

[marcelwgn]

This PR will add the API spec for the new ItemsSourceView.IndexOf function.

See microsoft/microsoft-ui-xaml#1828 for more context.

[MikeHillberg]

This is great -- I suggested some edits and added a couple of comments.

[marcelwgn]

So during testing of the existing internal IndexOf, we noticed one problem: Having a collection consisting only pass by value types such as integers, IndexOf does not work if we pass the collection as IIterable.

See following example:

var itemSourceView = new ItemsSourceView(Enumerable.Rang(0,10));

// The following is false!
// IndexOf(1) returns -1 
Assert.AreEqual(1,itemSourceView.IndexOf(1));

When passing pass by value types, they get boxed as objects. This happens both when creating an ItemsSourceView and when we call IndexOf. However those boxed objects will not be the same, so finding pass by value types using IndexOf fails.

The big question: Should we support it, or just point out in the documentation that this won't work?
Pass by value types are not a common scenario for ItemsSourceView, so is supporting them really necessary or beneficial?

[MikeHillberg]

Good point. This case is actually handled in the analogous ItemCollection.IndexOf, and similarly by Selector.SelectedItem, by unboxing and checking for the fundamental value types and for interfaces.

I don't know if there's any public API that exposes that functionality though. Does TabView.SelectedItem have code for it?

[marcelwgn]

Since the TabView uses a ListView internally, we do whatever ListView does to keep this up to date.

The main problem I see with doing this manually, is that we would have to unbox all the time, pushing down performance. Since the ItemsRepeater that uses this class, is mainly designed for large collections, this could really make a significant difference. And since most of the time, people actually pass objects and not primitive types, I am not sure if it's worth dropping performance for this "corner case".

If somebody wants to use IndexOf with pass by value types, they could use a different collection. We only would have a say in this if we get passed an IIterable, in other cases we don't even have an option on what we do, we just pass arguments down. So we might just say:

Pass by value types are not supported, if you want to use them use ... as itemssource.

[ranjeshj]

Since the TabView uses a ListView internally, we do whatever ListView does to keep this up to date.

The main problem I see with doing this manually, is that we would have to unbox all the time, pushing down performance. Since the ItemsRepeater that uses this class, is mainly designed for large collections, this could really make a significant difference. And since most of the time, people actually pass objects and not primitive types, I am not sure if it's worth dropping performance for this "corner case".

If somebody wants to use IndexOf with pass by value types, they could use a different collection. We only would have a say in this if we get passed an IIterable, in other cases we don't even have an option on what we do, we just pass arguments down. So we might just say:

Pass by value types are not supported, if you want to use them use ... as itemssource.

Right. It is quite common for someone to 'try' an enumerable of integers, however in most cases, the collection is going to be a non-value type. we still have the out of saying, if you want to use its, use a collection of some sort (like observablecollection) and In that case we will delegate to the collection. I agree that we should not be doing extra work for this case that might impact perf. I have not checked, but ListView likely has the same issue...

[MikeHillberg]

ListView likely has the same issue

ListView actually works, both SelectedItem and Items.IndexOf. The other case boxing shows up is in a list of structs, like a List<Point>. This will be easier with WinUI3 where we could share the code that ListView uses more easily. Maybe doc it until then.

[marcelwgn]

ListView actually works, both SelectedItem and Items.IndexOf. The other case boxing shows up is in a list of structs, like a List. This will be easier with WinUI3 where we could share the code that ListView uses more easily. Maybe doc it until then.

I've updated the spec to point out how pass by value types behave and how one can use them.

[ranjeshj]

ListView actually works, both SelectedItem and Items.IndexOf. The other case boxing shows up is in a list of structs, like a List. This will be easier with WinUI3 where we could share the code that ListView uses more easily. Maybe doc it until then.

I've updated the spec to point out how pass by value types behave and how one can use them.

I did not realize that ListView was working. I think we should make it work here also. I am ok with creating an issue and following up on that since it does not affect the API surface. I can see structs being used in this scenario where this can be a problem.

[marcelwgn]

ListView actually works, both SelectedItem and Items.IndexOf. The other case boxing shows up is in a list of structs, like a List. This will be easier with WinUI3 where we could share the code that ListView uses more easily. Maybe doc it until then.

I've updated the spec to point out how pass by value types behave and how one can use them.

I did not realize that ListView was working. I think we should make it work here also. I am ok with creating an issue and following up on that since it does not affect the API surface. I can see structs being used in this scenario where this can be a problem.

So should we support pass by value in the ItemsSourceView then? In that case, I think I could also fix this with the PR that we already have in WinUI that would add that API.

[ranjeshj]

ListView actually works, both SelectedItem and Items.IndexOf. The other case boxing shows up is in a list of structs, like a List. This will be easier with WinUI3 where we could share the code that ListView uses more easily. Maybe doc it until then.

I've updated the spec to point out how pass by value types behave and how one can use them.

I did not realize that ListView was working. I think we should make it work here also. I am ok with creating an issue and following up on that since it does not affect the API surface. I can see structs being used in this scenario where this can be a problem.

So should we support pass by value in the ItemsSourceView then? In that case, I think I could also fix this with the PR that we already have in WinUI that would add that API.

Let's follow up with a separate PR for that specific change. We could possibly share some of this code in WinUI3. Thanks.

[marcelwgn]

So is this spec then fine as it currently is or should I revert my last commit adding a comment regarding pass by value?

[MikeHillberg]

Thank you @chingucoding!