How to seperate commands out of big viewmodel

Question

My viewmodel contains lots of commands, it makes my viewmodel very big. I want to seperate my command out of the viewmodel. Currently, My soluton is create a class for each command like below,

 public class TestCommand : CommandBase
{
    private MainViewModel vm;

    public TestCommand(MainViewModel vm)
    {
        this.vm = vm;
    }

    public override bool CanExecute(object parameter)
    {
        return true;
    }

    public override void ExecuteCommand(object parameter)
    {
        vm.logger.log(...);
        ...
    }
}

Since I need to use some method or properties in ViewModel, so I have to pass the viewmodel as parameter to the command. For this solution, there're two disadvantages: 1. There're lots of command files in the project, if the average count of the command in one view is 15, 10 views will have 150 command files in the project; 2. Passing ViewModel as parameter to command needs some properties or methods which should be private have to been change to public; Also it's very strange to pass viewmodel to command.

Is there any other solution to seperate commands?

Answer 1

TL;DR:

ViewModels are presentation logic which is expressed mostly in commands, so it's not unusual that the commands take a big chunk of ViewModel's code. Don't try too hard to make the ViewModels as plain data holders (like common in ASP.NET MVC when using ViewModels) with INotifyPropertyChanged.

Long version

With the lack of more details it's hard to give you concrete tips, but here are some general guidelines. You may update your question with more details about the kind of commands you are using and I'll try to update the question.

1. Presentation Logic

   Main concern of ViewModels is presentation. There is no place for business Logic in the ViewModels.

   Business logic has to be extracted to either your business/domain models (if you follow the rich domain model) or services (in anemic domain model). In rich domain model your service layers are usually quite thin and mostly there to coordinate actions between multiple models.

   So if your ViewModel/commands are doing any kind of logic not related to presentation (if click button A, disable Buttons B, C and D or hide GroupBoxA or "disable button A if data is missing (CanExecute of ICommand)) it's probably doing too much.

2. Separation of concerns:

   It may be possible that your ViewModel tries to do more than it's intended to do. The logger in your example is such a hint. Logging is not a ViewModel concern.

   ViewModel is about presentation and presentation logic and logging is an application concern (as it do not belong to domain/business logic).

   Often a ViewModel can be split into two or more ViewModels (i.e. A ViewModel that manages a list of Customers and allow to edit the selected customer, usually can be split into 2 or 3 ViewModels: CustomersViewModel (display list), CustomerDetailViewModel or CustomerViewModel (detail to the customer) and a CustomerEditViewModel (to edit the customer in question)

   Concerns like logging and caching should be done using i.e. Decorator pattern. This requires that your services and/or repositories properly use and implement interfaces, then you can create decorators for caching or logging and rather than inject the original instance of your service, you implement the decorator.

   Dependency Injection (DI) and Inversion of Control (IoC) containers really help you with this. Having to manually wire this up (aka poor mans DI) is pain in the butt. Concrete examples are out of the scope of this answer.

3. Business Logic in Commands

   Commands shouldn't contain business logic. When your commands contain too much code (usually more than 5-20 lines of code) this is a good clue, that your commands may do too much.

   Commands should really only wire up multiple calls to services and assign data to Properties and/or rise events/messages (that are relevant to the presentation layer. Not to be confused with domain events, which shouldn't be raised inside commands). They are similar to "Actions" in MVC (like used frameworks in ASP.NET MVC for example).

   Commands should usually look something like this

   var customer = new Customer { Name = this.CustomerName, Mail = this.CustomerMail };
   try {
       this.customerService.AddCustomer(customer);
       // Add it to Observable<Customer> list so the UI gets updated
       this.Customers.Add(customer);
       // the service should have populated the Id field of Customer when persisting it
       // so we notify all other ViewModels that a new customer has been added
       this.messageBus.Publish(new CustomerCreated() { CustomerId = customer.Id } );
   } catch (SomeSpecificException e) { // Handle the Exception } 
   

   or

   this.Customers = this.customerRepository.GetAll();
   // Or this for async commands
   this.Customers = await this.customerRepository.GetAllAsync();
   

4. Encapsulation

   Many commands are very tightly coupled to the ViewModel itself and need access to internal state of the the ViewModel or the Model (the Model shouldn't be directly exposed to the View, this couples the Model to the View and any change in the model breaks your View and bindings).

   Moving these ICommands out the ViewModels may be difficult without breaking encapsulation.

Of course you can also implement multiple commands in one class

public class MyViewModelCommandHandler
{
    private readonly IMyRepository myRepository;

    public MyViewModelCommandHandler(/* pass dependencies here*/)
    {
        // assign and guard dependencies

        MyCommand = new RelayCommand(MyCommand, CanExecuteMyCommand);
        MyOtherCommand = new RelayCommand(MyOtherCommand, CanExecuteMyOtherCommand);
    }

    public ICommand MyCommand { get; protected set; } 
    public ICommand MyOtherCommand { get; protected set; } 

    private void MyCommand() 
    {
        // do something
    }

    private void CanExecuteMyCommand() 
    {
        // validate
    }

    private void MyOtherCommand() 
    {
        // do something else
    }

    private void CanExecuteMyOtherCommand() 
    {
        // validate
    }
}

And in your ViewModel simply assign these commands

public class MyViewModel : ViewModelBase 
{
    public MyViewModel()
    {
        var commandHandler = new MyCommandHandler(this);
        OneCommand = commandHandler.MyCommand;
        OtherCommand = commandHandler.MyOtherCommand;
    }

    public ICommand OneCommand { get; private set; } 
    public ICommand OtherCommand { get; private set; } 
}

You can also inject your MyCommandHandler into your view using an IoC container, this require remodeling your command handler class a bit, to create the ICommand on demand. Then you may use it like

public class MyViewModel : ViewModelBase 
{
    public MyViewModel(MyCommandHandler commandHandler)
    {
        OneCommand = commandHandler.CreateMyCommand(this);
        OtherCommand = commandHandler.CreateMyOtherCommand(this);
    }

    public ICommand OneCommand { get; private set; } 
    public ICommand OtherCommand { get; private set; } 
}

But that just shifts your problem, it won't solve the points 1. to 5. though. So I'd suggest to try the suggestions from the aboves list first and if your commands still contain "too many lines of code", try the other solution.

I don't like it too much as it creates unnecessary abstractions for little gain.

It's not uncommon that the ViewModels mainly consist of presentation logic, as that's their purpose and presentation logic is typically inside commands. Except that, you just have properties and the constructor. Properties shouldn't have anything else other than checking if value changed, then assign and one or more OnPropertyChanged calls.

So 50-80% of your ViewModel is code from commands.

Answer 2

Check if your view model can be divided into logical blocks and create sub-viewmodels for each blocks. The extra advantage is that these smaller viewmodels can be often reused when you want to display the same information in a different way somewhere else.

Also I prefer to have a general RelayCommand definition and just creating the commands in my viewmodel without specifying the different methods so I can keep the Execute and CanExecute together as lambda-expressions.

If creating different viewmodels isn't possible you can also split the code of your class over multiple files (partial classes) to increase maintainability.