worlds-simplest-csharp-wpf-mvvm-example icon indicating copy to clipboard operation
worlds-simplest-csharp-wpf-mvvm-example copied to clipboard

Business logic in ViewModel

Open placetobejohan opened this issue 4 years ago • 2 comments

In the blog post it says:

Model: This can be really simple, the goal here is for the ViewModel not to have to do any of the business logic.

However, the Presenter class - the ViewModel - seems to contain the business logic to convert text to upper case when creating a TextConverter. Shouldn't this be in the Model already?

Please note that I'm fairly new to MVVM so I could easily be wrong here.

placetobejohan avatar Apr 16 '20 14:04 placetobejohan

I agree. The AddToHistory should probably also be in the Model.

I think the ideal version would look something like:

IEnumerable<string> History => _model.History;
ICommand ConvertTextCommand => new Command(_ => _model.ConvertText(SomeText));

I might even go as far as having SomeText reference the model too with:

string SomeText
{
  get => _model.SomeText;
  set => UpdateValue(_model.SomeText, value);
}

Then the only field in the ViewModel would be a reference to Model _model. Giving a final version like:

public class Presenter : ObservableObject
{
    private readonly Model _model;

    public Presenter(Model model)
    {
        _model = model;
    }

    public string SomeText
    {
        get => _model.SomeText;
        set => UpdateValue(_model.SomeText, value);
    }

    public IEnumerable<string> History => _model.History;

    public ICommand ConvertTextCommand => new DelegateCommand(_model.ConvertText);
}

A little bit more work would be required for notification of changes to the UI.

MarkWithall avatar Apr 16 '20 15:04 MarkWithall

I've refactored to something similar to the above: https://github.com/MarkWithall/worlds-simplest-csharp-wpf-mvvm-example/tree/c%238.0

MarkWithall avatar Apr 17 '20 13:04 MarkWithall