Reputation: 9750
When using MVVM Community Toolkit, C#, .NET and Avalonia UI, I find myself making my model objects observable because it solves a recurring problem that I encounter when editing a model value from multiple controls.
However, I think that is bad practice. There is a GitHub Repo supporting this question, https://github.com/danieljfarrell/QuestionApp1, it's an Avalonia UI application that demonstrates the issue.
In a nutshell...
I like to define model objects with simple properties,
namespace QuestionApp1.Models
{
public class VolumeSetting
{
public int Volume { set; get; }
public VolumeSetting(int volume)
{
Volume = volume;
}
}
}
A single control can edit it's value fine.
However, I often want multiple controls to be able to bind-to and mutate the value of the model object's attributes.
In this case I tend to make the model object observable,
public partial class ObservableVolumeSetting : ObservableObject
{
[ObservableProperty]
public int volume;
public ObservableVolumeSetting(int volume)
{
Volume = volume;
}
}
The change notification will now keep values synchronised even when the value mutation occurs from multiple controls.
I have heard that this bad practice!
Very grateful for your advice.
Upvotes: 2
Views: 653
Reputation: 2675
Observable properties are just properties with a specialized change notification event. Depending on the intended use of the model, there is nothing wrong with using this approach. It isn't a bad practice at all to use events or even Callback methods in the model, that's how it supposed to be done in MVVM. There are only 3 real important tenants in MVVM:
What you do have to be mindful of is not allowing the view to subscribe to these events. If the view is directly aware of the model, you've lost the isolation that MVVM gives you.
Often times when I need that particular behavior, I will use my own events rather than implement INotfiyPropertyChanged. While the overall behavior is no different, the view can't really react to any other events (assuming binding), so it discourages the improper use of the model. The downside to this approach, is you can't use the convenient generator attributes in the CommunityToolkit to do this. However, there is nothing wrong with implementing INotifyPropertyChanged on any class.
Upvotes: 1
Reputation: 37
Making your model objects observable might seem like a convenient solution to handle updates from multiple controls, but it's often considered bad practice in the context of MVVM for a few reasons.
The main issue with making your models observable is that it tightly couples your model to the UI framework and introduces unnecessary complexity. Models should ideally represent your application's data and business logic, while the ViewModel acts as an intermediary between the model and the view. Mixing UI-specific logic like INotifyPropertyChanged (wich is done by your inheritance of ObservableObject) into your models breaks the separation of concerns.
Instead of making your models observable, you might want to keep them simple, and move the change notification logic to the view model. Here's how i personally would do it:
Plain model:
public class VolumeSetting
{
public int Volume { get; set; }
public VolumeSetting(int volume)
{
Volume = volume;
}
}
ViewModel for Notifications:
public class VolumeSettingViewModel : ObservableObject
{
private readonly VolumeSetting _volumeSetting;
public int Volume
{
get => _volumeSetting.Volume;
set
{
if (_volumeSetting.Volume != value)
{
_volumeSetting.Volume = value;
OnPropertyChanged(nameof(Volume));
}
}
}
public VolumeSettingViewModel(VolumeSetting volumeSetting)
{
_volumeSetting = volumeSetting;
}
}
You may also use some 3rd party libraries, like you did with ObservableObject. I think you're using CommunityToolkit?
Upvotes: 0