maui-samples icon indicating copy to clipboard operation
maui-samples copied to clipboard

[ObservableProperty] is not needed for ObservableCollection<T>, right?

Open suugbut opened this issue 10 months ago • 1 comments
trafficstars

I think [ObservableProperty] is not needed for ObservableCollection<T>.

https://github.com/dotnet/maui-samples/blob/4d9d1f3834d146a04b749fbf224978107a1e3031/9.0/Beginners-Series/BeginnersTask/ViewModel/MainViewModel.cs#L17

I propose the following:

public MainViewModel(IConnectivity connectivity)
{
        this.connectivity = connectivity;
}
public ObservableCollection<string> Items { get; } = [];

instead of

public MainViewModel(IConnectivity connectivity)
{
        Items = [];
        this.connectivity = connectivity;
}
[ObservableProperty]
ObservableCollection<string> items;

suugbut avatar Dec 27 '24 17:12 suugbut

Yeah it seems a bit odd. I guess it happened because adding that will generate a property through the MVVM Toolkit. In this case it's not necessarily wrong either because we assign the initial value somewhere later and this ensures that the data-binding is done properly.

Remember the ObservableCollection only notifies for changes within the collection itself, not when the Items property value has changed.

I don't see a strong reason to change it, but if you're willing to make a PR and show that everything still works with that as its maybe a slightly better practice I'm happy to review it :)

Thanks!

jfversluis avatar Jan 06 '25 19:01 jfversluis