ICommand with ViewModel dependency

Question

I am searching a pattern to keep SOLID principles in my application when I use ICommand. Basically my problem is the command execution has a dependency with the view model but at the same time the view model has a dependency with the command (I inject them by constructor). I would like to keep the viewmodel with properties only, so this is an example of my current implementation:

public class MyViewModel : INotifyPropertyChanged
{
   public ICommand MyCommand { get; private set; }

   public string Message { get; set; } // PropertyChanged ommited

   public MyViewModel()
   {            
   }

   public void SetCommand(ICommand myCommand)
   {
       this.MyCommand = myCommand;
   }

   ....
}

internal interface IMyViewModelCommandManager
{
    void ExectueMyCommand();
}

internal class MyViewModelCommandManager : IMyViewModelCommandManager
{
   private readOnly MyViewModel myViewModel;

   public MyViewModelCommandManager(MyViewModel myViewModel)
   {
       this.myViewModel = myViewModel;
   }

   public ExectueMyCommand()
   {
        MessageBox.Show(this.myViewModel.Message);
   }
}

internal class MyViewModelFactory: IMyViewModelFactory
{
   private readonly IContainerWrapper container;

   public MyViewModelFactory(IContainerWrapper container)
   {
      this.container = container;
   }

   public MyViewModel Create()
   {
       MyViewModel viewModel = new MyViewModel();

       IMyViewmodelCommandManager manager = this.container.Resolve<IMyViewmodelCommandManager>(new ResolverOverride[] { new ParameterOverride("viewModel", viewModel) });

       ICommand myCommand = new DelegateCommand(manager.ExecuteMyCommand);

       viewModel.SetCommand(myCommand);

       return viewModel;
   }
}

So, to avoid use the SetCommand method. I have thought two solutions but I don't know if they are elegant.

The first one is to move the viewmodel dependency from the constructor to the method updating the code in this way:

public class MyViewModel : INotifyPropertyChanged
{
   public ICommand MyCommand { get; private set; }

   public string Message { get; set; } // PropertyChanged ommited

   public MyViewModel(ICommand myCommand)
   {
       this.MyCommand = myCommand;            
   }

   ....
}

internal interface IMyViewModelCommandManager
{
    void ExectueMyCommand(MyViewModel viewModel);
}

internal class MyViewModelCommandManager : IMyViewModelCommandManager
{
   public MyViewModelCommandManager()
   {
       ....
   }

   public ExectueMyCommand(MyViewModel viewModel)
   {
        MessageBox.Show(myViewModel.Message);
   }
}

internal class MyViewModelFactory: IMyViewModelFactory
{
   private readonly IContainerWrapper container;

   public MyViewModelFactory(IContainerWrapper container)
   {
      this.container = container;
   }

   public MyViewModel Create()
   {
       IMyViewmodelCommandManager manager = this.container.Resolve<IMyViewmodelCommandManager>(..);

       ICommand myCommand = new DelegateCommand<MyViewModel>(manager.ExecuteMyCommand);

       MyViewModel viewModel = new MyViewModel(myCommand);
       return viewModel;
   }
}

Of course, the xaml code will use CommandParameter:

<Button Content="Show Message" Command="{Binding MyCommand}" CommandParameter="{Binding .}" />

Other solution I have thought is to use a trick creating a Wrapper of the viewModel and the commandManager have a dependency with the Wrapper instead of the viewModel:

internal class MyViewModelCommandContext
   {
      public MyViewModel ViewModel { get; set; }
   }

   public class MyViewModel : INotifyPropertyChanged
    {
       public ICommand MyCommand { get; private set; }

       public string Message { get; set; } // PropertyChanged ommited

       public MyViewModel(ICommand myCommand)
       {
           this.MyCommand = myCommand;            
       }

       ....
    }

    internal interface IMyViewModelCommandManager
    {
        void ExectueMyCommand();
    }

    internal class MyViewModelCommandManager : IMyViewModelCommandManager
    {
       private readonly MyViewModelCommandContext context;

       public MyViewModelCommandManager(MyViewModelCommandContext context)
       {
           this.context = context;
           ....
       }

       public ExectueMyCommand()
       {
            MessageBox.Show(this.context.myViewModel.Message);
       }
    }

    internal class MyViewModelFactory: IMyViewModelFactory
    {
       private readonly IContainerWrapper container;

       public MyViewModelFactory(IContainerWrapper container)
       {
          this.container = container;
       }

       public MyViewModel Create()
       {
           MyViewModelCommandContext context = new MyViewModelCommandContext();

           IMyViewmodelCommandManager manager = this.container.Resolve<IMyViewmodelCommandManager>(new ResolverOverride[] { new ParameterOverride("context", context) });

           ICommand myCommand = new DelegateCommand(manager.ExecuteMyCommand);

           MyViewModel viewModel = new MyViewModel(myCommand);
           context.ViewModel = viewModel;
           return viewModel;
       }
    }

In my opinion the first one is the best solution for this problem, what do you think is the best solution. Would you apply another solution?

Answer 1

IMHO both solutions are overly complicated. SOLID is great, KISS is better.

Your MyViewModelCommandManager is currently directly coupled to MyViewModel since it needs the latter's Message, so what is the advantage of having them separate? Why not simply implement the command inside MyViewModel?

If this would entail injecting too many dependencies into MyViewModel, then think about what you actually need the command to do, and abstract away everything else that isn't needed.

• The command displays a message.
• The message is held by MyViewModel
• You want to display the message outside MyViewModel (maybe other viewmodels also need to display a message and you want to reuse the code?)
• So all you really need is some kind of notification from MyViewModel that it wants to display a message, or that something has occured that would result in a message being displayed.

Possible solutions:

1. Inject an IMessageDisplayService to MyViewModel. MyViewModel calls it with the message.
2. Inject a callback to MyViewModel similar to above.
3. Have MyViewModel raise an event with the message as an EventArg.

The inferred responsibilities of the above solutions are subtly different.

1. means that MyViewModel is in charge. It wants to display a message.
2. is less explicit. MyViewModel knows it needs to call the callback, but doesn't really know or care what it does.
3. is like 2 but even more decoupled. Multiple things can subscribe or unsubscribe to the event but MyViewModel remains blissfully ignorant.

All three of these mean that the thing displaying the message has no need to know about MyViewModel. You have decoupled them. It's MyViewModelFactory that does any wiring up required.

Answer 2

Thanks for your opinions.

I understand you when you say I am creating a complex pattern, but in a big project with a big developers team, if there is not clear patterns with split responsabilities, the code maintenance could be impossible to perform.

Reading you and your third solution I have thought one possible solution. It seems complexity but, in my opinion, improves the code quality. I will create a commandContext, which only have the viewmodel properties needed for the code, avoiding to have all viewmodel in the command manager. Also I will create a class whose responsability is to mantain the context updated when the viewmodel changes. This is the possible code:

internal class MyCommandContext
{
    public string Message { get; set; }
}

public class MyViewModel : INotifyPropertyChanged
{
    public ICommand MyCommand { get; private set; }

    public string Message { get; set; } // PropertyChanged ommited

    public string OtherProperty { get; set; }

    public ObservableCollection<MyChildViewModel> Childs { get; set; }

    public MyViewModel(ICommand myCommand)
    {
        this.MyCommand = myCommand;            
    }

       ....
}

internal interface IMyViewModelCommandManager
{
    void ExectueMyCommand();
}

internal class MyViewModelCommandManager : IMyViewModelCommandManager
{
   private readonly MyCommandContext context;

   public MyViewModelCommandManager(MyViewModelCommandContext context)
   {
       this.context = context;
       ....
   }

   public ExectueMyCommand()
   {
        MessageBox.Show(this.context.Message);
   }
}

internal interface IMyViewModelCommandSynchronizer
{
    void Initialize();
}

internal class MyViewModelCommandSynchronizer : IMyViewModelCommandSynchronizer, IDisposable
{
     private readOnly MyViewModel viewModel;
     private readOnly MyCommandContext context;

     MyViewModelCommandSynchronizer(MyViewModel viewModel, MyCommandContext context)
     {
         this.viewModel = viewModel;
         this.context = context;
     }

     public void Initialize()
     {
         this.viewModel.PropertyChanged += this.ViewModelOnPropertyChanged;
     }

    private void ViewModelOnPropertyChanged(object sender, PropertyChangedEventArgs e)
    {
        if (e.PropertyName == "Message")
        {
             this.context.Message = this.viewModel.Message;
        }
    }

    // Dispose code to deattach the events.
}

internal class MyViewModelFactory: IMyViewModelFactory
{
   private readonly IContainerWrapper container;

   public MyViewModelFactory(IContainerWrapper container)
   {
       this.container = container;
   }

   public MyViewModel Create()
   {
       MyCommandContext context = new MyCommandContext();

       IMyViewmodelCommandManager manager = this.container.Resolve<IMyViewmodelCommandManager>(new ResolverOverride[] { new ParameterOverride("context", context) });

       ICommand myCommand = new DelegateCommand(manager.ExecuteMyCommand);

       MyViewModel viewModel = new MyViewModel(myCommand);

       IMyViewModelCommandSynchronizer synchronizer = this.container.Resolve<IMyViewmodelCommandManager>(new ResolverOverride[] { new ParameterOverride("context", context), new ParameterOverride("viewModel", viewModel) });

       synchronizer.Initialize();

       return viewModel;
   }
}