2
\$\begingroup\$

I have a need to get notified about validation errors that occur in a child collection of my ViewModel. For validation I use the INotifyDataErrorInfo interface.
I've created a class that handles this successfully however, I feel that additional refactoring / simplification can be done on it.
Here's the class that implements validation and child collection validation tracking:

public class Validatable : Observable, INotifyDataErrorInfo
{
    [XmlIgnore]
    private Dictionary<string, List<string>> errors =
            new Dictionary<string, List<string>>();

    [XmlIgnore]
    // a collection that keeps all child tracked collections
    public List<ICollection> TrackedCollections { get; set; } = new List<ICollection>();

    [XmlIgnore]
    public bool HasCollectionErrors { get; set; }

    // method used by the view model
    internal void RegisterCollection<T>(ObservableCollection<T> collection)
    {
        TrackedCollections.Add(collection);
        collection.CollectionChanged += Collection_CollectionChanged;
    }

    // handle tracked collections changed events and registeres / unregisteres the error changed events
    private void Collection_CollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        if (e.NewItems != null)
        {
            foreach (var item in e.NewItems.OfType<Validatable>())
            {
                item.ErrorsChanged += Item_ErrorsChanged;
            }
        }
        if (e.OldItems != null)
        {
            foreach (var item in e.OldItems.OfType<Validatable>())
            {
                item.ErrorsChanged -= Item_ErrorsChanged;
            }
        }
    }

    // notify parent about validation status change
    private void Item_ErrorsChanged(object sender, DataErrorsChangedEventArgs e)
    {
        HasCollectionErrors = false;
        foreach (var TrackedCollection in TrackedCollections)
        {
            foreach (var obj in TrackedCollection.OfType<Validatable>())
            {
                if (obj.HasErrors)
                {
                    HasCollectionErrors = true;
                    break;
                }
            }
        }
        OnPropertyChanged(nameof(IsValid));
    }

    public event EventHandler<DataErrorsChangedEventArgs> ErrorsChanged;

    public bool IsValid => !HasErrors && !HasCollectionErrors;
    public bool HasErrors => errors.Any();

    public void OnErrorsChanged(string propertyName)
    {
        ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(propertyName));
    }

    public IEnumerable GetErrors(string propertyName)
    {
        if (string.IsNullOrWhiteSpace(propertyName)) return null;
        if (errors.ContainsKey(propertyName) &&
            errors[propertyName] != null &&
            errors[propertyName].Count > 0)
        {
            return errors[propertyName];
        }
        return null;
    }

    public void Validate()
    {
        ClearErrors();

        var results = new List<ValidationResult>();
        var context = new ValidationContext(this);
        Validator.TryValidateObject(this, context, results, true);

        if (results.Any())
        {
            var propertyNames = results.SelectMany(m => m.MemberNames).Distinct().ToList();

            foreach (var propertyName in propertyNames)
            {
                errors[propertyName] = results
                   .Where(m => m.MemberNames.Contains(propertyName))
                   .Select(r => r.ErrorMessage)
                   .Distinct()
                   .ToList();
                OnErrorsChanged(propertyName);
            }
        }
        OnPropertyChanged(nameof(IsValid));
    }

    protected void ClearErrors()
    {
        foreach (var propertyName in errors.Keys.ToList())
        {
            errors.Remove(propertyName);
            OnErrorsChanged(propertyName);
        }
    }
}

Observable class:

public class Observable : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        var handler = PropertyChanged;
        if (handler != null)
        {
            handler(this, new PropertyChangedEventArgs(propertyName));
        }
    }

    protected virtual void SetProperty<T>(ref T member, T val, [CallerMemberName]string propertyName = null)
    {
        if (Equals(member, val)) return;

        member = val;
        if (PropertyChanged != null)
            PropertyChanged(this, new PropertyChangedEventArgs(propertyName));
    }
}

Sample View Model:

public class Person : Validatable
{
    private string _name;

    public Person()
    {
        Addresses = new ObservableCollection<Address>();
        RegisterCollection(Addresses);
    }

    [Required]
    public string Name
    {
        get { return _name; }
        set
        {
            SetProperty(ref _name, value);
            Validate();
        }
    }

    public ObservableCollection<Address> Addresses { get; private set; }
}

public class Address : Validatable
{
    private string _street;

    [Required]
    public string Street
    {
        get { return _street; }
        set
        {
            SetProperty(ref _street, value);
            Validate();
        }
    }
}
\$\endgroup\$
1
\$\begingroup\$

Validatable

In the Item_ErrorsChanged() method you have a naming issue in the foreach (var TrackedCollection in TrackedCollections) loop.

Local variables should be named using camelCase casing. TrackedCollection -> trackedCollection but maybe a different name would be better to distinguish it more from TrackedCollections.

This

protected void ClearErrors()
{
    foreach (var propertyName in errors.Keys.ToList())
    {
        errors.Remove(propertyName);
        OnErrorsChanged(propertyName);
    }
}  

is a little bit bloated

  • Clearing all of the keys and values of a dictionary should be done using the Clear() method of the Dictionary<TKey, TValue>
  • The call to ToList() on the KeyCollection is superflous

protected void ClearErrors()
{
    foreach (var propertyName in errors.Keys)
    {
        OnErrorsChanged(propertyName);
    }
    errors.Clear();
}  

Observable

You have some code duplication here. Why don't you call OnPropertyChanged() out of the SetProperty<T>() method ?

Your coding style gets inconsistent here. In the Validatable class you used the C# 6 feature using the null conditional operator ? and in this class you are checking against null.

You should change this class like so

public class Observable : INotifyPropertyChanged
{
    public event PropertyChangedEventHandler PropertyChanged;

    protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
    {
        PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
    }

    protected virtual void SetProperty<T>(ref T member, T val, [CallerMemberName]string propertyName = null)
    {
        if (Equals(member, val)) { return; }

        member = val;

        OnPropertyChanged(propertyName);
    }
}  

As you can see I have added braces {} to the if of the SetProperty() method as well. I am a defender of always use them.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.