Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
suugbut opened this issue Dec 27, 2024 · 1 comment
Open

Comments

@suugbut
Copy link

suugbut commented Dec 27, 2024

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

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;
@jfversluis
Copy link
Member

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants