C#•15mo ago

✅ Correct MVVM approach for displaying models representing settings

Given that I have a set of models representing settings, When I supply then to a UI utilizing MVVM, And display each settings with a respective UI component, Then the user can change the given setting And the change will be reflected in the model. As far as I've understood, Models shouldn't really be reactive, i.e., shouldn't implement the INotifyPropertyChanged interface. This goes inline with the motive to keep models very simple, containing primarily properties. Assume that I have generic models representing, e.g., a Toggle, Text input, Number input, and a Date input settings. They share the same interface ISettingsElement, and are exposed to the View via the respective ViewModel. Displaying specific UI elements based on the provided model is pretty easy. But how do I make sure that the model values are two-way-bound to the UI elements? E.g., I wish to display my collection of settings, and provide the user with a Clear or Defaults button. The ViewModel would naturally expose respective ICommand implementations to do exactly that. But... going through the Models and modifying their configured values won't be reflected in the UI. Or, is there a non-hacky way to tell the parent UI control to refresh itself? All this seems a bit hacky and limiting, when trying to do clean MVVM... Hopefully, you'll be able to provide me the missing puzzle piece for correctly understading MVVM and implementing the settings feature.

66 Replies

Azrael•15mo ago

Or, is there a non-hacky way to tell the parent UI control to refresh itself?

INotifyPropertyChanged Why wouldn't you implement the INotifyPropertyChanged interface? I've never heard anyone say that, might just be me though. Alternatively you can use ObservableCollection<T>.

https://learn.microsoft.com/en-us/dotnet/api/system.collections.objectmodel.observablecollection-1?view=net-7.0

DenisOP•15mo ago

So it is acceptable to have the Model observable? I've been trying to find a persons response to me in one of the posts, where they've claimed, that observable models break the cleanliness of MVVM But I wasn't able to find it...

Azrael•15mo ago

That's my opinion. I don't know about the best practice within MVVM but I use ObservableCollection<T> for my Models. INotifyPropertyChanged it's personal preference mostly. There's no right or wrong.

Azrael•15mo ago

You can wrap your Models in ViewModels.

https://stackoverflow.com/questions/25747962/wrapping-models-in-mvvm

Stack Overflow

Wrapping models in MVVM

This question has originally started at Code Review so to avoid repetition I won't paste it all here. So after I received an answer to my initial question I have another question: How should I wra...

Azrael•15mo ago

https://stackoverflow.com/questions/11773555/mvvm-how-to-wrap-viewmodel-in-a-viewmodel

Stack Overflow

MVVM - How to wrap ViewModel in a ViewModel?

First of all, I have read this post and did not find the answer for my problem. I am not sure if this is an aggregated Model class or an aggregated ViewModel class, but this is what I have: In my...

Azrael•15mo ago

I'd still recommend using a Model and ObservableCollection<T>.

DenisOP•15mo ago

Not sure what you mean by this Let's assume I have a model representing a checkbox setting

[ObservableObject]
public partial record CheckboxElement
  : INodeLabeledElement, INodeValueElement<bool>
{
  [ObservableProperty]
  private bool m_value;

  public bool DefaultValue { get; init; }

  /// <inheritdoc />
  public string Label { get; init; } = string.Empty;
}

[ObservableObject]
public partial record CheckboxElement
  : INodeLabeledElement, INodeValueElement<bool>
{
  [ObservableProperty]
  private bool m_value;

  public bool DefaultValue { get; init; }

  /// <inheritdoc />
  public string Label { get; init; } = string.Empty;
}

How do I use the ObservableCollection<T>?

Azrael•15mo ago

ObservableCollection<CheckboxElement> checkBoxElement = new ObservableCollection<CheckboxElement>();

Florian Voß•15mo ago

he didn't say you shouldnt implement it, he said you shouldn't implement it on the Model and I would agree on that should be implemented on the ViewModel IMO

Azrael•15mo ago

Oh I misread. Sorry. Yeah. https://discord.com/channels/143867839282020352/1146465716955004988/1146473522974175332 - Goes in the VM.

MODiX•15mo ago

Azrael

ObservableCollection<CheckboxElement> checkBoxElement = new ObservableCollection<CheckboxElement>();

Quoted by

<@242650820120805376> from #Correct MVVM approach for displaying models representing settings (click here)

React with ❌ to remove this embed.

Florian Voß•15mo ago

you're right on that viewmodel is responsible of managing the interaction between views and models. Models are just dumb data classes. Viewmodel should implement INotifyPropertyChanged to update the views with the model data via bindings

Azrael•15mo ago

Yep.

DenisOP•15mo ago

So what is the correct approach? The checkbox element contains a property with the current checked state, that value should be somehow cleanly bound to the View So I create a new ViewModel for each model instance?? this seems a bit overkill or am I missing the point?

Florian Voß•15mo ago

it is indeed overkill and you seem to be missunderstanding a viewmodel may contain lots of data / models I create one viewmodel per view a view may display lots of data

DenisOP•15mo ago

[ObservableObject]
public partial class MainWindowViewModel
{
  public IReadOnlyCollection<INodeElement> Settings { get; }

  /// <summary>
  /// Default constructor
  /// </summary>
  public MainWindowViewModel()
  {
    Settings = new INodeElement[]
    {
      new CheckboxElement
      {
        Label = "My checkbox",
        DefaultValue = true
      },
      new SettingSeparator(),
      new CheckboxElement
      {
        Label = "My checkbox",
        DefaultValue = false
      },
    };
  }

  [RelayCommand]
  public void Clear()
  {
  }

  [RelayCommand]
  public void Reset()
  {
  }
}

[ObservableObject]
public partial class MainWindowViewModel
{
  public IReadOnlyCollection<INodeElement> Settings { get; }

  /// <summary>
  /// Default constructor
  /// </summary>
  public MainWindowViewModel()
  {
    Settings = new INodeElement[]
    {
      new CheckboxElement
      {
        Label = "My checkbox",
        DefaultValue = true
      },
      new SettingSeparator(),
      new CheckboxElement
      {
        Label = "My checkbox",
        DefaultValue = false
      },
    };
  }

  [RelayCommand]
  public void Clear()
  {
  }

  [RelayCommand]
  public void Reset()
  {
  }
}

<Window>
  <Window.DataContext>
    <local:MainWindowViewModel />
  </Window.DataContext>
  <Grid>
    <ItemsControl ItemsSource="{Binding Settings, Mode=OneWay}">
      <ItemsControl.ItemsPanel>
        <ItemsPanelTemplate>
          <VirtualizingStackPanel />
        </ItemsPanelTemplate>
      </ItemsControl.ItemsPanel>
      <ItemsControl.Resources>
        <DataTemplate DataType="{x:Type settings:CheckboxElement}">
          <CheckBox Content="{Binding Label}" IsChecked="{Binding Value}" />
        </DataTemplate>

        <DataTemplate DataType="{x:Type settings:SettingSeparator}">
          <Border
            BorderBrush="Black"
            BorderThickness="1"
            Width="50" />
        </DataTemplate>
      </ItemsControl.Resources>
    </ItemsControl>
  </Grid>
</Window>

<Window>
  <Window.DataContext>
    <local:MainWindowViewModel />
  </Window.DataContext>
  <Grid>
    <ItemsControl ItemsSource="{Binding Settings, Mode=OneWay}">
      <ItemsControl.ItemsPanel>
        <ItemsPanelTemplate>
          <VirtualizingStackPanel />
        </ItemsPanelTemplate>
      </ItemsControl.ItemsPanel>
      <ItemsControl.Resources>
        <DataTemplate DataType="{x:Type settings:CheckboxElement}">
          <CheckBox Content="{Binding Label}" IsChecked="{Binding Value}" />
        </DataTemplate>

        <DataTemplate DataType="{x:Type settings:SettingSeparator}">
          <Border
            BorderBrush="Black"
            BorderThickness="1"
            Width="50" />
        </DataTemplate>
      </ItemsControl.Resources>
    </ItemsControl>
  </Grid>
</Window>

[ObservableObject]
public partial class CheckboxElement
  : INodeLabeledElement, INodeValueElement<bool>
{
  [ObservableProperty]
  private bool m_value;

  public bool DefaultValue { get; init; }

  /// <inheritdoc />
  public string Label { get; init; } = string.Empty;
}

[ObservableObject]
public partial class CheckboxElement
  : INodeLabeledElement, INodeValueElement<bool>
{
  [ObservableProperty]
  private bool m_value;

  public bool DefaultValue { get; init; }

  /// <inheritdoc />
  public string Label { get; init; } = string.Empty;
}

Here's what I have. What would be the most optimal approach, asusming I shouldn't make the CheckboxElement model observable?

Azrael•15mo ago

You're over complicating it.

DenisOP•15mo ago

that's good to hear

Florian Voß•15mo ago

public class MainWindowViewModel : INotifyPropertyChanged {
    private ObservableCollection<INodeElement> _settings; 
    public ObservableCollection<INodeElement> Settings { get => _settings; set { _settings = value; OnPropertyChanged(nameof(Settings)); } }
}

public class MainWindowViewModel : INotifyPropertyChanged {
    private ObservableCollection<INodeElement> _settings; 
    public ObservableCollection<INodeElement> Settings { get => _settings; set { _settings = value; OnPropertyChanged(nameof(Settings)); } }
}

now you can bind this thing to some xaml control and the ui should update when Settings changes note if you are on .Net FW and not on .Net (-Core) you will have to implement the interface correctly, in new .Net it has default implementation

DenisOP•15mo ago

On new . NET, fortunately.

Florian Voß•15mo ago

great so then it should work as I've send tho haven't tested

DenisOP•15mo ago

This kinda doesn't make sense to me, still.

Florian Voß•15mo ago

whats unclear?

DenisOP•15mo ago

Isn't an observable collection notifying the view of changes to the quantity of elements? Not the quality (properties within said elements)?

Florian Voß•15mo ago

no it updates everything you need ObservableCollections makes sure that when you add or remove items the UI will reflect that, the Setter makes sure that when you assign new collection UI also reflects that

DenisOP•15mo ago

Even if I do Settings.First().Value = "foobar"; That is clear to me

Florian Voß•15mo ago

I'd assume this to work, I'm not sure tho try it out

Azrael•15mo ago

Wouldn't INodeElement also need the INotifyPropertyChanged?

DenisOP•15mo ago

That's exactly what I always presumed

Florian Voß•15mo ago

possibly, I dunno tbh. Have you tried it out?

DenisOP•15mo ago

Unless somehow Observable collection is tracking hashes of its elements or whatever

Azrael•15mo ago

I'm pretty sure that it does. The model as it is doesn't get updated right now I think.

Florian Voß•15mo ago

try this now, without having INodeElement implement it, then we know

Azrael•15mo ago

Yep. @Denis You should use INotifyPropertyChanged. It's too much work and inefficient to force it manually. You'd also need to re-initialize the VM.

DenisOP•15mo ago

What why

Azrael•15mo ago

Because you need to force the collection to update itself. Maybe you can use UpdateSourceTrigger?

DenisOP•15mo ago

This sounds so wrong that my head is incapable of understanding it. So, to this I shall just say, ok 😂

Azrael•15mo ago

https://learn.microsoft.com/en-us/dotnet/api/system.windows.data.binding.updatesourcetrigger?view=windowsdesktop-7.0

Bindings that are TwoWay or OneWayToSource listen for changes in the target property and propagate them back to the source. This is known as updating the source.

DenisOP•15mo ago

Yes I'm aware of the update mode This might be interesting...

Florian Voß•15mo ago

so whats the outcome @Denis ?

DenisOP•15mo ago

Haven't tested yet, not at the pc. But from my previous research of the internals of Observable Collection it naturally would only notify the view of changes whenever it's item count is modified. The OC does not notify the view of internal changes of its items.

Azrael•15mo ago

^ As I said. That's my theory.

DenisOP•15mo ago

I could potentially for cases like Clear and SetDefaults call UpdateSourcr

Azrael•15mo ago

We won't know until you've tested it though.

DenisOP•15mo ago

Ik, just not at the pc rn

Florian Voß•15mo ago

given that this is for Settings, I think its fair to assume that this collection will never be large. So you could just create a new collection in local variable which is a copy of your prop and then reassign the property with new collection. This way you don't have to implement INotifyPropertyChanged on INodeElement and performance decrease should be irrelevant

DenisOP•15mo ago

This is not really a viable and scalable solution My question, even though that it comes from a specific example, primarily pokes at the philosophy of MVVM. The goal here is to find the correct way of handling data from models, that should be reactive If making models reactive is not an anti-pattern or whatever, then there is no issue with what I'm doing right now If however, models should stay dumb, then how are these issues handled? E.g., in cases where you have tons of data updating in real time Nope, only tracks changes to the element count AND when you replace an element at a given index with a new one So Add, Remove, [] = the items within ObservableCollection<T> are not tracked

Azrael•15mo ago

Thought so.

DenisOP•15mo ago

that would be pretty overkill, if it did

Azrael•15mo ago

Why don't you want to implement INotifyPropertyChanged?

Florian Voß•15mo ago

@Denis I guess the question would be what else are you looking for when Add, Remove, Replace or reassign isn't enough

DenisOP•15mo ago

when you update the item itself

Florian Voß•15mo ago

Replace

DenisOP•15mo ago

I do, but the question is whether it is the correct approach

Azrael•15mo ago

Just clear and add. There's no best or correct approach.

DenisOP•15mo ago

So imageine that you have a DataGrid with n items, where n can reach insane numbers. The items are updated in real time to, e.g., reflect the current price changes on the stock market. Don't think that Replace is the best approach... Then it is safe to assume, that reactive models in MVVM is fine, and I should have nothing to worry about

Azrael•15mo ago

Yes.

DenisOP•15mo ago

yay

Azrael•15mo ago

Wait what do you mean by this?

MODiX•15mo ago

Azrael

ObservableCollection<CheckboxElement> checkBoxElement = new ObservableCollection<CheckboxElement>();

Quoted by

<@242650820120805376> from #Correct MVVM approach for displaying models representing settings (click here)

React with ❌ to remove this embed.

Azrael•15mo ago

Like this right?

DenisOP•15mo ago

that a model can inherit INotifyPropertyChanged and notify the view, which binds to a given property of them model, of any changes this

Azrael•15mo ago

You could, but why? What's the point?

DenisOP•15mo ago

In this code snippet, I have a CheckboxElement model that has a property Value, that is observable -> changes to it are sent to the view e.g., I wish to have the view reflect the changes into the model. I wish to change values of the models from the viewmodel for them to be reflected in the view Maybe create some complex conditions, where enabling one checkbox setting, would disable a section of other settings

Azrael•15mo ago

Yeah.

DenisOP•15mo ago

So, there is a point, OR am I missing some much simplier and straightforward solution? 😄 Clearing and readding all elements/replacing is not an option in my mind

Gaming

Programming

✅ Correct MVVM approach for displaying models representing settings