You've received two other answers already. Unfortunately, neither precisely addresses both the Add and Remove commands. Also, one prefers to focus primarily on code-behind implementation rather than XAML declarations, and is fairly sparse on details anyway, while the other more-correctly focuses on implementation in XAML where appropriate, but does not include correct, working code, and (slightly) obfuscates the answer by introducing the extra abstraction of the RelayCommand type.
So, I will offer my own take on the question, with the hopes this will be more useful to you.
While I agree that abstracting the ICommand implementation into a helper class such as RelayCommand is useful and even desirable, unfortunately this tends to hide the basic mechanisms of what's going on, and requires a more elaborate implementation that was offered in the other answer. So for now, let's ignore that.
Instead, just focus on what does need to be implemented: two different implementations of the ICommand interface. Your view model will expose these as the values of two bindable properties representing the commands to be executed.
Here is a new version of your ViewModel class (with the irrelevant and unprovided ObservableObject type removed):
class ViewModel
{
private class AddCommandObject : ICommand
{
private readonly ViewModel _target;
public AddCommandObject(ViewModel target)
{
_target = target;
}
public bool CanExecute(object parameter)
{
return true;
}
public event EventHandler CanExecuteChanged;
public void Execute(object parameter)
{
_target.AddContentItem();
}
}
private class RemoveCommandObject : ICommand
{
private readonly ViewModel _target;
public RemoveCommandObject(ViewModel target)
{
_target = target;
}
public bool CanExecute(object parameter)
{
return true;
}
public event EventHandler CanExecuteChanged;
public void Execute(object parameter)
{
_target.RemoveContentItem((TabItem)parameter);
}
}
private ObservableCollection<TabItem> tabItems;
public ObservableCollection<TabItem> TabItems
{
get { return tabItems ?? (tabItems = new ObservableCollection<TabItem>()); }
}
public ICommand AddCommand { get { return _addCommand; } }
public ICommand RemoveCommand { get { return _removeCommand; } }
private readonly ICommand _addCommand;
private readonly ICommand _removeCommand;
public ViewModel()
{
TabItems.Add(new TabItem { Header = "One", Content = DateTime.Now.ToLongDateString() });
TabItems.Add(new TabItem { Header = "Two", Content = DateTime.Now.ToLongDateString() });
TabItems.Add(new TabItem { Header = "Three", Content = DateTime.Now.ToLongDateString() });
_addCommand = new AddCommandObject(this);
_removeCommand = new RemoveCommandObject(this);
}
public void AddContentItem()
{
TabItems.Add(new TabItem { Header = "Three", Content = DateTime.Now.ToLongDateString() });
}
public void RemoveContentItem(TabItem item)
{
TabItems.Remove(item);
}
}
Note the two added nested classes, AddCommandObject and RemoveCommandObject. These are both examples of nearly the simplest implementation of ICommand possible. They can always be executed, and so the return value of CanExecute() never changes (so there's no need to ever raise the CanExecuteChanged event). They do need the reference to your ViewModel object so that they can each call the appropriate method.
There are also two public properties added to allow binding of these commands. Of course, the RemoveContentItem() method needs to know what item to remove. This needs to be set up in the XAML, so that the value can be passed as a parameter to the command handler and from there to the actual RemoveContentItem() method.
In order to support the use of the keyboard for the commands, one approach is to add input bindings to the window. This is what I've chosen here. The RemoveCommand binding additionally needs the item to be deleted to be passed as the command parameter, so this is bound to the CommandParameter for the KeyBinding object (just as for the CommandParameter of the Button in the item).
The resulting XAML looks like this:
<Window.DataContext>
<data:ViewModel/>
</Window.DataContext>
<Window.InputBindings>
<KeyBinding Command="{Binding AddCommand}">
<KeyBinding.Gesture>
<KeyGesture>Ctrl+N</KeyGesture>
</KeyBinding.Gesture>
</KeyBinding>
<KeyBinding Command="{Binding RemoveCommand}"
CommandParameter="{Binding SelectedItem, ElementName=tabControl1}">
<KeyBinding.Gesture>
<KeyGesture>Ctrl+W</KeyGesture>
</KeyBinding.Gesture>
</KeyBinding>
</Window.InputBindings>
<Grid>
<Grid.RowDefinitions>
<RowDefinition Height="Auto"/>
<RowDefinition Height="*"/>
</Grid.RowDefinitions>
<TabControl x:Name="tabControl1" ItemsSource="{Binding TabItems}" Grid.Row="1" Background="LightBlue">
<TabControl.ItemTemplate>
<DataTemplate>
<StackPanel Orientation="Horizontal" VerticalAlignment="Center">
<TextBlock Text="{Binding Header}" VerticalAlignment="Center"/>
<Button Content="x" Width="20" Height="20" Margin="5 0 0 0"
Command="{Binding DataContext.RemoveCommand, RelativeSource={RelativeSource AncestorType=TabControl}}"
CommandParameter="{Binding DataContext, RelativeSource={RelativeSource Self}}">
</Button>
</StackPanel>
</DataTemplate>
</TabControl.ItemTemplate>
<TabControl.ContentTemplate>
<DataTemplate>
<TextBlock Text="{Binding Content}" />
</DataTemplate>
</TabControl.ContentTemplate>
</TabControl>
</Grid>
EDIT:
As I mentioned above, there is in fact benefit to abstracting the ICommand implementation, using a helper class instead of declaring a new class for every command you want to implement. The referenced answer at Why RelayCommand mentions loose coupling and unit testing as motivations. While I agree these are good goals, I can't say that these goals are in fact served per se by the abstraction of the ICommand implementation.
Rather, I see the benefits as being the same ones primarily found when making such abstractions: it allows for code reuse, and in doing so improves developer productivity, along with code maintainability and quality.
In my above example, every time you want a new command, you have to write a new class that implements ICommand. On the one hand, this means that each class you write can be tailor-made to the specific purpose. Dealing with CanExecuteChanged or not, as the case requires, passing parameters or not, etc.
On the other hand, every time you write such a class, that's an opportunity to write a new bug. Worse, if you introduce a bug which is then later copy/pasted, then when you eventually find the bug, you may or may not fix it everywhere it exists.
And of course, writing such classes over and over gets tedious and time-consuming.
Again, these are just specific examples of the general conventional wisdom of the "best practice" of abstracting reusable logic.
So, if we've accepted that an abstraction is useful here (I certainly have :) ), then the question becomes, what does that abstraction look like? There are a number of different ways to approach the question. The referenced answer is one example. Here is a slightly different approach that I've written:
class DelegateCommand<T> : ICommand
{
private readonly Func<T, bool> _canExecuteHandler;
private readonly Action<T> _executeHandler;
public DelegateCommand(Action<T> executeHandler)
: this(executeHandler, null) { }
public DelegateCommand(Action<T> executeHandler, Func<T, bool> canExecuteHandler)
{
_canExecuteHandler = canExecuteHandler;
_executeHandler = executeHandler;
}
public bool CanExecute(object parameter)
{
return _canExecuteHandler != null ? _canExecuteHandler((T)parameter) : true;
}
public event EventHandler CanExecuteChanged;
public void Execute(object parameter)
{
_executeHandler((T)parameter);
}
public void RaiseCanExecuteChanged()
{
EventHandler handler = CanExecuteChanged;
if (handler != null)
{
handler(this, EventArgs.Empty);
}
}
}
In the ViewModel class, the above would be used like this:
class ViewModel
{
private ObservableCollection<TabItem> tabItems;
public ObservableCollection<TabItem> TabItems
{
get { return tabItems ?? (tabItems = new ObservableCollection<TabItem>()); }
}
public ICommand AddCommand { get { return _addCommand; } }
public ICommand RemoveCommand { get { return _removeCommand; } }
private readonly ICommand _addCommand;
private readonly ICommand _removeCommand;
public ViewModel()
{
TabItems.Add(new TabItem { Header = "One", Content = DateTime.Now.ToLongDateString() });
TabItems.Add(new TabItem { Header = "Two", Content = DateTime.Now.ToLongDateString() });
TabItems.Add(new TabItem { Header = "Three", Content = DateTime.Now.ToLongDateString() });
// Use a lambda delegate to map the required Action<T> delegate
// to the parameterless method call for AddContentItem()
_addCommand = new DelegateCommand<object>(o => this.AddContentItem());
// In this case, the target method takes a parameter, so we can just
// use the method directly.
_removeCommand = new DelegateCommand<TabItem>(RemoveContentItem);
}
Notes:
- Of course, now the specific
ICommand implementations are no longer needed. The AddCommandObject and RemoveCommandObject classes have been removed from the ViewModel class.
- In their place, the code uses the
DelegateCommand<T> class.
- Note that in some cases, the command handler is not going to need the parameter passed to the
ICommand.Execute(object) method. In the above, this is addressed by accepting the parameter in a lambda (anonymous) delegate, and then ignoring it while calling the parameterless handler method. Other ways to approach this would be to have the handler method accept the parameter but then ignore it, or to have a non-generic class in which the handler delegate itself can be parameterless. IMHO, there's no "right way" per se…just various choices that may be considered more or less preferable according to personal preference.
- Note also that this implementation differs from the referenced answer's implementation in the handling of the
CanExecuteChanged event. In my implementation, the client code is given fine-grained control over that event, at the expense of requiring the client code to retain a reference to the DelegateCommand<T> object in question and calling its RaiseCanExecuteChanged() method at the appropriate time. In the other implementation, it instead relies on the CommandManager.RequerySuggested event. This is a trade-off of convenience vs efficiency and, in some cases, correctness. That is, it is less convenient for the client code to have to retain references to commands which may change the executable status, but if one goes the other route, at the very least the CanExecuteChanged event may be raised much more often than is required, and in some cases it's even possible it may not be raised when it should have been (which is far worse than the possible inefficiency).
On that last point, yet another approach would be to make the ICommand implementation a dependency object, and provide a dependency property that is used to control the executable state of the command. This is a lot more complicated, but overall could be considered the superior solution as it allows fine-grained control over the CanExecuteChanged event's raising, while providing a good, idiomatic way to bind the executable state of the command, e.g. in XAML to whatever property or properties actually determine said executability.
Such an implementation might look something like this:
class DelegateDependencyCommand<T> : DependencyObject, ICommand
{
public static readonly DependencyProperty IsExecutableProperty = DependencyProperty.Register(
"IsExecutable", typeof(bool), typeof(DelegateCommand<T>), new PropertyMetadata(true, OnIsExecutableChanged));
public bool IsExecutable
{
get { return (bool)GetValue(IsExecutableProperty); }
set { SetValue(IsExecutableProperty, value); }
}
private static void OnIsExecutableChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
DelegateDependencyCommand<T> command = (DelegateDependencyCommand<T>)d;
EventHandler handler = command.CanExecuteChanged;
if (handler != null)
{
handler(command, EventArgs.Empty);
}
}
private readonly Action<T> _executeHandler;
public DelegateDependencyCommand(Action<T> executeHandler)
{
_executeHandler = executeHandler;
}
public bool CanExecute(object parameter)
{
return IsExecutable;
}
public event EventHandler CanExecuteChanged;
public void Execute(object parameter)
{
_executeHandler((T)parameter);
}
}
In the above, the canExecuteHandler argument for the class is eliminated, in lieu of the IsExecutable property. When that property changes, the CanExecuteChanged event is raised. IMHO it's unfortunate that there is this discrepancy in the ICommand interface between how it's designed and how WPF normally works (i.e. with bindable properties). It's a bit weird that we have essentially a property but which is exposed via an explicit getter method named CanExecute().
On the other hand, this discrepancy does serve some useful purposes, including making explicit and convenient the use of the CommandParameter for both the execution of the command, and the checking for executability. These are worthwhile goals. I'm just not sure personally whether I'd have made the same choice balancing them with the consistency of the usual way state is connected within WPF (i.e. through binding). Fortunately, it's simple enough to implement the ICommand interface in a bindable way (i.e. as above), if that's really desired.
