Where do I put the logic for my ICommand?

Question

I've recently started using the MVVM pattern in silverlight, and i'm not sure if i am using it correctly.

GUI

I currently have a MainView that has combobox of stock market sectors. When the user selects a sector (eg ENERGY) and clicks the Add button a list of stocks for that sector are displayed in a listbox. By the side of each stock in the listbox is a remove button that allows you to remove the individual stock from the listbox.

I have implemented the following ViewModels. (Below is just an indication of the code)

public class MainViewModel
{
  public SectorViewModel CurrentSector 
  {
     get;
     set;
  }

  public string SelectedSector 
  {
     get;
     set;
  }


  public void AddSectorClickedCommand()
  {
     CurrentSector = new SectorViewModel(SelectedSector);
  }

}

public class SectorViewModel
{
     public ObservableCollection<StockViewModel>  Stocks = new ObservableCollection<StockViewModel>();

     public SectorViewModel(string sector)
     {
        List<Stocks> stocklist = StockProvider.GetStocks(sector);
        for each (var s in stocklist)
        {
            StockViewModel svm = new StockViewModel(s);
            svm.Remove+= { //Remove svm from Stocks collection logic
            Stocks.add(svm);

        }
     }
}

My question is; in whcih viewmodel is it best to add the code implementation for the Remove button of each row in the listbox?? The Remove button should remove the StockViewModel from the SectorViewModel.Stocks collection.

I have currently added the RemoveClicked method to the StockViewModel(as shown above). This code fires an event back to the SectorViewModel and the RemoveStock method of the SectorViewModel removes the StockViewModel from the Stock collection.

Is there a better way to implement this remove functionality? I'm new to MVVM and am not sure if this is the best approach to develop this functionility, since the SectorViewModel needs to register to events of a StockViewModel.

Answer 1

Personally I don't like events because you should unsubscribe from them and also they can be used where it isn't appropriate.

I would use the constructor parameter to handle the remove command, something like this:

public class StockViewModel
{
    public StockViewModel(Stock stock, Action<StockViewModel> removeCommandAction)
    {
        //...
        this.RemoveCommand = new DelegateCommand(() => removeCommandAction(this));
    }
}

public class SectorViewModel
{
    public SectorViewModel()
    {
        //...
        StockViewModel svm = new StockViewModel(s, this.RemoveStock);
        Stocks.add(svm);
    }

    private void RemoveStock(StockViewModel stock)
    {
        //...
    }
}

Another approach is to use some kind of the EventAggregator pattern, for example, the Messenger class from the MVVM light Toolkit. But I think that it is an overkill for such simple task:

public StockViewModel(Stock stock, IMessenger messenger)
{
    //...
    this.RemoveCommand = new DelegateCommand(() => 
        messenger.Send(new NotificationMessage<StockViewModel>(this, RemoveItemNotification)));
}

public SectorViewModel(IMessenger messenger)
{
    //...
    messenger.Register<NotificationMessage<StockViewModel>>(this, msg =>
    {
        if (msg.Notification == StockViewModel.RemoveItemNotification)
        {
            this.RemoveStock(msg.Content);
        }
    }
}

Also I heard that Silverlight 5 supports binding to a relative source. So there is the 3rd approach. I'm not sure whether this example works, but at least it should:

<Button Content="Remove" 
    Command="{Binding DataContext.RemoveCommand RelativeSource={RelativeSource AncestorType=ListBox}}"
    CommandParameter="{Binding}" />

public class SectorViewModel
{
    public SectorViewModel()
    {
        this.RemoveCommand = new DelegateCommand(obj => this.RemoveStock((StockViewModel)obj));
    }

    public ICommand RemoveCommand { get; set; }
}

The last example is the most preferred by the way and is used in WPF applications because WPF has always had RelativeSource binding.