MVVM logic architecture

Question

I've got a problem with designing good wrchitecture for my MVVM project. I came up with something like below, please read and tell me if this is wrong approach or ok: I will have some interface:

 public interface ISomeStrategy
        {
            void LoadData();

            void Search(string text);

            IList<SomeObject> SomeObjectsList { get; set; }
        }

And a class that implements this interface:

 public class FirstStrategy: INotifyPropertyChanged, ISomeStrategy
    {
        public CompositePayslipItem(IDataService dataService)
        {
         DataService = dataService;
        }
     private IDataService DataService;
     public IList<SomeObject> SomeObjectList{get; set;}

     public async void LoadData()
     {
       SomeObjectList =await DataService.GetAll();
     }

     public async void Search(string text)
     {
       SomeObjectList =await DataService.GetByKey(text);
     }

}

And ViewModel:

public void SomeViewModel
{
   public SomeViewModel(IDataService dataService)
   {
    Strategy = new FirstStrategy(dataService);
    Strategy.LoadData();
   }
   public ISomeStrategy Strategy {get; set;}
   private Command<string> searchCommand;

   public Command<string>  SearchCommand => searchCommand?? (searchCommand= new Command(ExecuteSearchCommandAsync));


  private async void ExecuteSearchCommandAsync(string text)
  {
    Strategy.Search(text);
  }
}

As you can see all logics will be in "Strategy" classes which will be binded via ViewModel to View. What it gives to me? I can dynamicly change implementation at runtime. For example if I have a Employee and Manager, where logic for search and getting data is different I can just implement another class and change property ISomeStrategy Strategy without editing existing code for Employee ( no additional if's in a lot of methods). Unfortunetly there is a few problems with it:

1. All bindings (except Commands) will be in class *Factory which can be misleading
2. In most cases one VM will have one Strategy implementation so there will be a lot of code for nothing. On the other hand, in the future client can demand another role (implemenation) and it will be easier (just implement another class - no editing old code).

What do you think about it? Or maybe you using another patterns for business logic?

EDIT: Xaml part:

<ContentPage
    x:Class="Test.SomePage"
    xmlns="http://xamarin.com/schemas/2014/forms"
    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml">
                <StackLayout>
                    <SearchBar
                        HeightRequest="50"
                        SearchCommand="{Binding SearchCommand}" />
                    <ListView
                        x:Name="formatsList"
                        Margin="0"
                        ItemsSource="{Binding Strategy.SomeObjectList}">
                </StackLayout>
</ContentPage>

Answer 1

In an attempt to help you, and hopefully I understood your question:

All bindings (except Commands) will be in class Factory which can be misleading: I assume you bind to the viewmodel and not directly to the strategy.

You should bind to the view model, from there interact with your strategy. Otherwise you are mixing your layers.

update

As far as I know, not an issue and a typical programmer should be able to spot it instantly.

If SomeObject is a kind of model from the business layer, you are breaking the separation of layers argument. Future developers might not see the consequences of e.g. renaming a property in SomeObject. If SomeObject is part of the view-models than you're safe. Please mind; I tend to over-engineer things ;-)

So you might consider to create a dedicated viewmodel for SomeObject and use an automapper to map to the business related model.

In most cases one VM will have one Strategy implementation so there will be a lot of code for nothing. On the other hand, in the future client can demand another role (implemenation) and it will be easier (just implement another class - no editing old code).

I can't find a question or problem here. As a general rule, using design patterns take a bit of more effort upfront, to help you deal with the ever changing requirements later on.

For more info; see this (my XD) post: ViewModels in MVC / MVVM / Seperation of layers- best practices?

---

Some more advice on your code, disclaimer: some are opinion based:

---

Loading data in a constructor is bad practice.

A typical load action should be non-blocking and async to keep UI responsive.

public SomeViewModel(IDataService dataService)
{
   //load data in constructor issue
   Strategy.LoadData();
}

---

You are creating a new object if is null. A race condition might occur if multiple threads are calling this property. Consider a lock (perhaps with a explicit set method), or initialize in the constructor. I think this isn't really an issue in your code since you mentioned Factory, still:

Calling new uses concretes, not to interfaces. Use a factory or IoC (with a resolver, i.e. factory) to overcome this problem.

public SomeViewModel(IDataService dataService)
{
    //`new` issue
    Strategy = new FirstStrategy(dataService);
}

---

Do not use async void prefer async Task

See async/await - when to return a Task vs void?

//use Task if possible
private async void ExecuteSearchCommandAsync(string text)

---

Possible (tricky) race condition

In this line:

public Command<string>  SearchCommand => 
   searchCommand?? (searchCommand= new Command(ExecuteSearchCommandAsync));