WPF command binding with input validation - how to

Question

In my ViewModel I have implemented IDataErrorInfo interface (along with INotifyPropertyChanged). Input validation works as intended, I have no problems there.

I have this property as part of IDataErrorInfo public string Error { get { return this[null]; } } To my understanding, Error should be empty if all validated inputs pass validation, so I pass this as my CanExecute method

return !string.IsNullOrEmpty(Error);

But, my "save" button never gets enabled. My gues is that CanExecuteChanged never gets trigered. If that's true, where and how should I trigger it?

---

This is my RelayCommand class. I have tried other ways of implementation, but the results were the same. I think it works, because the "save" button is enabled if I don't pass CanExecute method to the constructor.

public class RelayCommand : ICommand
{
    private readonly Action execute;
    private readonly Func<bool> canExecute;

    public RelayCommand(Action execute, Func<bool> canExecute = null)
    {
        this.execute = execute;
        this.canExecute = canExecute;
    }

    public bool CanExecute(object parameter)
    {
        return canExecute == null || canExecute();      
    }

    public event EventHandler CanExecuteChanged
    {
        add { CommandManager.RequerySuggested += value; }
        remove { CommandManager.RequerySuggested -= value; }
    }

    public void Execute(object parameter) { execute(); }
}

The "save" button:

<Button Content="Save" Command="{Binding InsertCommand}"/>

InsertCommand:

public RelayCommand InsertCommand { get; internal set; }

In the ViewModel constructor:

InsertCommand = new RelayCommand(ExecuteInsert, CanExecuteInsert);

CanExecute:

bool CanExecuteInsert()
{
    return !string.IsNullOrEmpty(Error);
}

Answer 1

You haven't really added enough code for us to accurately tell you what your problem is. However, you are taking the correct approach. I also use the IDataErrorInfo interface, but I added some extra properties into my base class that implements it:

public string Error // actual IDataErrorInfo Member
{
    get
    {
        if (!HasError) return string.Empty;
        StringBuilder errors = new StringBuilder();
        foreach (string error in Errors) errors.AppendUniqueOnNewLineIfNotEmpty(error);
        return errors.ToString();
    }
}

public virtual ObservableCollection<string> Errors
{
    get { return errors; }
}

public virtual bool HasError
{
    get { return Errors != null && Errors.Count > 0; }
}

The Errors collection just enables me to maintain multiple errors simultaneously and HasError just tells me if there are any errors or not. The Errors collection is filled using the IDataErrorInfo indexer in each data type:

public override ObservableCollection<string> Errors
{
    get
    {
        errors = new ObservableCollection<string>();
        errors.AddUniqueIfNotEmpty(this["Title"]);
        errors.AddUniqueIfNotEmpty(this["Artist"]);
        ...
        errors.AddUniqueIfNotEmpty(this["DealerPrice"]);
        return errors;
    }
}

So to answer your actual question, I would handle the CanExecute functionality of the Save Command like this:

    public override ICommand Save
    {
        get { return new ActionCommand(action => SaveCommand(), canExecute => 
            CanSave(DigitalServiceProviderPriceTier)); }
    }
...
    private bool CanSave(DigitalServiceProviderPriceTier digitalServiceProviderPriceTier)
    {
        return digitalServiceProviderPriceTier != null && 
            digitalServiceProviderPriceTier.HasChanges && 
            !digitalServiceProviderPriceTier.HasError; // <-- Important part
    }

So, it seems as though you are doing this in almost the same way - my additional properties are of course optional. If your Error property is never empty, then I would say that that is your problem. Start by debugging it and seeing what value it actually has... perhaps there is always an error there that shouldn't be?

Ahhhh... I just noticed your Error property code... that is your problem:

public string Error { get { return this[null]; } } 

You are calling the indexer with a value of null, so what the code in your indexer is returning is actually what your Error property value will be. An indexer should also return an empty string if there are no validation errors:

public override string this[string propertyName]
{
    get
    {
        string error = string.Empty;
        if (propertyName == "SomePropertyName" && SomePropertyName.IsNullOrEmpty()) 
            error = "You must enter some property.";
        if (propertyName == "OtherPropertyName" && OtherPropertyName.Length != 3) 
            error = "The OtherPropertyName must be 3 characters long.";
        ...
        return error;
    }
}

Then in your Error property, you should call the actual property names that you want to validate as I did in my Errors property and not null. So in the example above, you could call something like this in your Error property:

string error = this["SomePropertyName"];
if (error == string.Empty) error = this["OtherPropertyName"];
return error;

Once again, I've written too much information... I just hope it all makes sense to you and you're not going to return with dozens of new questions. Hopefully, it's clear enough.