0

(This is probably a dupe, but if so I haven't seen it yet, although it may be Best practice for encapsulating a parameter that requires multiple interfaces to be implemented, but that question doesn't answer my situation)

Normally I don't care that c# only has single inheritance, but I have run into something where I think that multiple inheritance actually would improve things, and I can't see a clean way to do it with single inheritance.

This comes about from wanting to support both INotifyPropertyChanged and INotifyDataErrorInfo in a re-usable manner. These interfaces are independent and require a non-trivial amount of code to support them, and up until now I have only considered INotifyPropertyChanged and have a system of:

public class MyPropertyChangedBase : INotifyPropertyChanged
{
   // re-usable code to support INotifyPropertyChanged
}

public MyViewModel : MyPropertyChangedBase
{
  // various WPF bound properties that notify the UI on change
}

However, now I want to implement INotifyDataErrorInfo in a re-usable manner, but the code to support this interface is totally orthogonal and unrelated to the INotifyPropertyChanged code. So ideally they should be implemented in unrelated base classes and use multiple inheritance for my view model. EG (note this is NOT valid c# code - it is only for highlighting an 'idealistic' situation):

public class MyPropertyChangedBase : INotifyPropertyChanged
{
   // re-usable code to support INotifyPropertyChanged
}

public class MyErrorChangedBase : INotifyDataErrorInfo
{
   // re-usable code to support INotifyDataErrorInfo
}

public MyViewModel : MyPropertyChangedBase, MyErrorChangedBase
{
  // various WPF bound properties that notify the UI on change and error
}

(Note that I can't use composition to fix this, as I would need to map from the components to the public instances of the interface elements, which just shifts the problem and doesn't solve it)

So what I seem to be left with is having either the INotifyPropertyChanged code rely on the INotifyDataErrorInfo code or vice versa. EG

public class MyPropertyChangedBase : INotifyPropertyChanged
{
   // re-usable code to support INotifyPropertyChanged
}

public class MyPropertyAndErrorChangedBase : MyPropertyChangedBase , INotifyDataErrorInfo
{
   // re-usable code to support INotifyDataErrorInfo
}

public MyViewModel : MyPropertyAndErrorChangedBase
{
  // various WPF bound properties that notify the UI on change and error
}

This reliance of one independent class on another doesn't sit well with me. So is there a cleaner way to do it? Or am I stuck with it?


Finally, I do recognize that realistically you would not implement INotifyDataErrorInfo without first having implemented INotifyPropertyChanged, so the reliance of one class on the other may be a pragmatic tradeoff. But I am still trying to find the cleanest way of doing this.

  • If you recognize that realistically/practically you wouldn't implement INotifyDataErrorInfo without INotifyPropertyChanged then your question is a bit moot :). A different specific example might have a different answer (like composition, as you mentioned). – 17 of 26 Nov 9 '17 at 13:49
  • Also, coming from a C++ background I can tell you that multiple inheritance winds up being more trouble than it's worth. – 17 of 26 Nov 9 '17 at 13:51
0

As Greg has already mentioned, your two interfaces relate to the View. More specifically, they both relate to ViewModel<->View interactions. So, having one class implement both is not a big deal.

I do, however, understand that you might want to evolve the behaviour of each interface implementation independently, so how about this composition-based approach:

public abstract class MyPropertyChangedBase : INotifyPropertyChanged
{
    // re-usable code to support INotifyPropertyChanged
}

public abstract class MyErrorChangedBase : INotifyDataErrorInfo
{
    // re-usable code to support INotifyDataErrorInfo
}

public abstract class ViewModelBase : MyPropertyChangedBase
{
    // classic VM
}

public abstract class ErrorAwareViewModelBase : INotifyPropertyChanged, INotifyDataErrorInfo
{
    private readonly MyPropertyChangedBase changeNotifier;
    private readonly MyErrorChangedBase errorNotifier;

    public ErrorAwareViewModelBase(MyPropertyChangedBase changeNotifier, MyErrorChangedBase errorNotifier)
    {
        this.changeNotifier = changeNotifier;
        this.errorNotifier = errorNotifier;
    }

    event PropertyChangedEventHandler INotifyPropertyChanged.PropertyChanged
    {
        add
        {
            this.changeNotifier.PropertyChanged += value;
        }

        remove
        {
            this.changeNotifier.PropertyChanged -= value;
        }
    }

    bool INotifyDataErrorInfo.HasErrors => this.errorNotifier.HasErrors;

    event EventHandler<DataErrorsChangedEventArgs> INotifyDataErrorInfo.ErrorsChanged
    {
        add
        {
            this.errorNotifier.ErrorsChanged += value;
        }

        remove
        {
            this.errorNotifier.ErrorsChanged -= value;
        }
    }

    IEnumerable INotifyDataErrorInfo.GetErrors(string propertyName)
    {
        return this.errorNotifier.GetErrors(propertyName);
    }
}
  • OK I see where you are going with an adapter class. This is probably more along the lines I was thinking. One thought I did have about your solution is that I think that the ErrorAwareViewModelBase constructor only needs to be injected with interfaces and not concrete classes. – Peter M Nov 9 '17 at 14:44
  • That's the approach I went with initially (see edit history) but I thought it might not be explicit enough that you'd be using your existing implementations. – MetaFight Nov 9 '17 at 19:10
1

I see absolutely no reason why implementing both INotifyPropertyChanged and INotifyDataErrorInfo on a parent class is bad design. Both interfaces deal with the view. These things are tightly coupled anyway. You gain nothing but pain of maintenance by separating them into different classes, especially if it means exposing private or protected state in a public context in order to achieve this separation.

Separation of Concerns that requires breaking Encapsulation ends up being a Scattering of Concerns, because any arbitrary code can change that private state.

Go with one base class that implements two interfaces.

Jumping from C# and the Windows Presentation Foundation framework, I've seen a number of code examples in Java where a data access object implements two different interfaces:

public interface BlogDataReader {
    Blog find(int blogId);
}

public interface BlogDataWriter {
    void create(Blog blog);
    void update(Blog blog);
}

public interface BlogDataDeleter {
    void remove(Blog blog);
}

public class BlogDataAccess implements BlogDataReader, BlogDataWriter, BlogDataDeleter {
    public Blog find(int blogId) {
        // ...
    }

    public void create(Blog blog) {
        // ...
    }

    public void update(Blog blog) {
        // ...
    }

    public void remove(Blog blog) {
        // ...
    }
}

A class, or base class, implementing multiple related interfaces is not an anti-pattern or bad design.

  • I know where you are coming from, but I don't believe that your last statement is a fair representation of the situation. Yes, both interfaces deal with the view, but the concepts they represent are totally orthogonal. So I would restate it as A class, or base class, implementing multiple un-related blocks of code is a bad design, because putting all the code in the one base class intertwines it and you now rely on the good graces of the coder not to screw up and change one interface implementation when he meant to change the other. – Peter M Nov 9 '17 at 14:37
0

In general, I think my stab at it would be with extension methods or helper classes. Rather than put the code into a base class, move it into extension methods. Something like the following:

public static class INotifyPropertyChangedExtensions
{
    public static ThingToImplementFirstNotifyMethod(INotifyPropertyChanged this myObj)
    {
        /...
    }
}
public static class INotifyDataErrorInfoExtensions
{
    public static ThingToImplementFirstErrorMethod(INotifyPropertyChanged this myObj)
    {
        /...
    }
}

then in your class do something like this:

public class MyClass : INotifyPropertyChanged, INotifyDataErrorInfo
{
    public void INotifyPropertyChangedFirstMethod()
    {
        this.ThingToImplementFirstNotifyMethod()
    }
    public void INotifyDataErrorInfoFirstMethod()
    {
        this.ThingToImplementFirstErrorMethod()
    }
}

If the code required to implement the interface takes more state, a helper class and composition might be better:

public class INotifyPropertyChangedHelper
{
    private int state;
    public ThingToImplementFirstNotifyMethod(INotifyPropertyChanged this myObj)
    {
        /...
    }
}
public class INotifyDataErrorInfoHelper
{
    private int state;
    public ThingToImplementFirstErrorMethod(INotifyPropertyChanged this myObj)
    {
        /...
    }
}

and your class would look like this:

public class MyClass : INotifyPropertyChanged, INotifyDataErrorInfo
{
    private INotifyPropertyChangedHelper _notifyHelper;
    private INotifyDataErrorInfoHelper _errorHelper;
    public MyClass()
    {
        _notifyHelper = new INotifyPropertyChangedHelper();
        _errorHelper = new INotifyDataErrorInfoHelper()
    }
    public void INotifyPropertyChangedFirstMethod()
    {
        _notifyHelper.ThingToImplementFirstNotifyMethod()
    }
    public void INotifyDataErrorInfoFirstMethod()
    {
        _errorHelper.ThingToImplementFirstErrorMethod()
    }
}

If the interfaces are related closely, doing something like what @GregBurghardt suggests would work as well.

  • My problem with your solution is that now I need to manually inject the same code into all of my view model classes - and I am trying to minimize that sort of thing. The less code I have to duplicate the better! – Peter M Nov 9 '17 at 14:49
  • @PeterM Agreed, but as you've noted, without MI you are kind of up a creek here. I'm assuming the code for implementing the interface is non-trivial and as such, using the extension methods or the adapter-esque composition idea would help in not repeating the heavy lifting code. Otherwise you have to provide a base class that implements both interfaces. And then that code becomes unavailable to an object that only wants to implement just one interface. And if you do something like the other answers that have NotifyPropertyChangeBase and NotifyAndErrorPropertyBase then you have (cont) – Becuzz Nov 9 '17 at 16:00
  • @PeterM a more common base you can use (NotifyPropertyChangeBase) other places, but all the code to implement the (presumably) less common error interface becomes hidden within the class that implements both. If you want to keep that available for something else, it has to get moved out to a separate accessable place. You could harness one of the ideas that I outlined above and make 3 base classes (NotifyBase, ErrorBase and NotifyAndErrorBase) which use the extension methods. Then just inherit from the appropriate one. (cont) – Becuzz Nov 9 '17 at 16:01
  • @PeterM This has the disadvantage that as the number of interfaces increases the number of base classes increases factorial-ly. In any case, I don't think you are going to get away from either implementing both interfaces in a common base or adding some amount of duplication. It becomes a judgement call as to what is going to be more maintainable for you based on your use case. – Becuzz Nov 9 '17 at 16:03

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.