[Core] Isn't DataTemplateSelector the wrong name?

[adrianknight89]

This class is inheriting from DataTemplate which technically makes it another data template. A selector seems to imply a different kind of class that provides functionality around the object it works with. Shouldn't it be actually called something like MultiDataTemplate? I'm wondering if this class can be marked obsolete in favor of something that serves its purpose. Maybe the name is fine, but it shouldn't inherit from that base class.

[andreinitescu]

I can't understand why DataTemplateSelector derives from DataTemplate.
I wonder for example what happens if I have a DataTemplateSelector instance and I call CreateContent() ?
Following other XAML technologies, the DataTemplateSelector in Xamarin Forms should not derive from any other class.

[GalaxiaGuy]

This happens: #3544

[andreinitescu]

@GalaxiaGuy So much confusion in that thread, and it shows that having DataTemplateSelector derived from DataTemplate creates a lot of confusion. I am not sure why the issue is marked with "needs-repro" and "needs-info".

The method CreateContent actually guards against using DataTemplateSelector in place of DataTemplate, because really a DataTemplateSelector is not what a DataTemplate is meant for:
https://github.com/xamarin/Xamarin.Forms/blob/master/Xamarin.Forms.Core/ElementTemplate.cs#L79

I think what happened is, in Xamarin Forms someone decided to fuse the roles of DataTemplate and DataTemplateSelector, in order to make it easier™ to work with ListView.
Instead, it should really be a separate specialized property of DataTemplateSelector type, just how the XAML frameworks (WPF, UWP) do it (See ItemTemplateSelector in UWP)

[GalaxiaGuy]

I assume the tags are left there from the initial post which didn't have a concrete example of the problem.

One thing to note is it not just ListView using DataTemplate these days. The new Shell stuff uses it as well as a handful of third party libraries - including one I wrote which is why I have familiarity with it.

I faced the issue of just calling CreateContent on a DataTemplate variable and expecting it to work, but then see it fail when it turns out that variable actually holds a DataTemplateSelector. Putting whether they should have been different aside (and I suspect changing now may bring more problems), I don't see why CreateContent on DataTemplateSelector can't be made to just work. Move the code from the extension method into ElementTemplate (or make CreateContent virtual and override it).

There is also the limitation that DataTemplateSelector can't return another DataTemplateSelector which seems a bit arbitrary.

Of course there is the issue that changing any code that interacts with ListView should probably be considered high risk...

[andreinitescu]

One thing to note is it not just ListView using DataTemplate these days. The new Shell stuff uses it as well as a handful of third party libraries - including one I wrote which is why I have familiarity with it.

I think you're missing the point, there's no problem with using DataTemplate. Like I said in previous message, the problem is with DataTemplateSelector class which (only) in Xamarin Forms derives from DataTemplate. Having it this way created a lot of confusion and problems. Just this check should had raised a lot of red flags.

In XAML frameworks (WPF\UWP) DataTemplateSelector and DataTemplate each have a separate role, otherwise why have both?

[GalaxiaGuy]

I was trying to point that even though having ItemTemplate be a single property that could either be a DataTemplate or a DataTemplateSelector (and essentially forcing one to inherit from the other) is not a great design decision, there is more than ListView now depending on this behavior. Adding a separate ItemTemplateSelector property like in WPF to ListView would be a solution, but the other style expecting a template selector to be passed in the DataTemplate property would still exist in third party code.

Other than the idea that changing it now might cause more problems, I agree with what you said.

[andreinitescu]

Adding a separate ItemTemplateSelector property like in WPF to ListView would be a solution, but the other style expecting a template selector to be passed in the DataTemplate property would still exist in third party code.

Adding the new property would not break existing code, it would just give the right way which should have existed from the beginning.
The new property would become the right way, while using ItemTemplate for selector would become depracated (just add logging a message )
The existing implementation breaks a fundamental design principle, Liskov substitution principle

[andreinitescu]

@xamarin Your feedback is needed, how do you want to handle this?

Unfortunately changing DataTemplate not derive fromDataTemplateSelector would be ideal, but it's not possible now obviously, because it will break every app.

In order to make it a bit better for existing ListView, my suggestion would be to:

1. Add a new property to ListView (similar to how UWP\WPF has it, see here):

   public DataTemplateSelector ItemTemplateSelector { get; set; }
   

2. Deprecate setting a DataTemplateSelector to ItemTemplate property of the ListView. For this, add a log message in a couple of places in the ListView native renderers, warning developer to use ItemTemplateSelector property instead.

I hope at least the new CollectionView will have a dedicated ItemTemplateSelector.

[StephaneDelcroix]

maybe the name is wrong, maybe the inheritance tree is wrong... but this ship has sailed, and we can't change that anymore :/

let's follow the discussion on how to open it in #3544