Search code examples
c#wpfmvvmliskov-substitution-principle

Does this violate the Liskov substitution principle, and if so, what do I do about it?


Use case: I'm using data templates to match a View to a ViewModel. Data templates work by inspecting the most derived type of the concrete type provided, and they don't look at what interfaces it provides, so I have to do this without interfaces.

I'm simplifying the example here and leaving out NotifyPropertyChanged, etc., but in the real world, a View is going to bind to the Text property. For simplicity, imagine that a View with a TextBlock would bind to a ReadOnlyText and a View with a TextBox would bind to WritableText.

class ReadOnlyText
{
    private string text = string.Empty;

    public string Text
    {
        get { return text; }
        set
        {
            OnTextSet(value);
        }
    }

    protected virtual void OnTextSet(string value)
    {
        throw new InvalidOperationException("Text is readonly.");
    }

    protected void SetText(string value)
    {
        text = value;
        // in reality we'd NotifyPropertyChanged in here
    }
}

class WritableText : ReadOnlyText
{
    protected override void OnTextSet(string value)
    {
        // call out to business logic here, validation, etc.
        SetText(value);
    }
}

By overriding OnTextSet and changing its behavior, am I violating the LSP? If so, what's a better way to do it?


Solution

  • LSP states that a subclass should be substiutable for it's superclass (see stackoverflow question here). The question to ask yourself is, "Is writeable text a type of readonly text?" The answer is clearly "no", in fact these are mutually exclusive. So, yes, this code violates LSP. However, is writable text a type of readable text (not readonly text)? The answer is "yes". So I think the answer is to look at what it is you're trying to do in each case and possibly to change the abstraction a bit as follows:

    class ReadableText
    {
        private string text = string.Empty;
        public ReadableText(string value)
        {
            text = value;
        }
    
        public string Text
        {
            get { return text; }
        }
    }          
    
    class WriteableText : ReadableText
    {
        public WriteableText(string value):base(value)
        {
    
        }
    
        public new string Text
        {
            set
            {
                OnTextSet(value);
            }
            get
            {
                return base.Text;
            }
        }
        public void SetText(string value)
        {
            Text = value;
            // in reality we'd NotifyPropertyChanged in here       
        }
        public void OnTextSet(string value)
        {
            // call out to business logic here, validation, etc.       
            SetText(value);
        }
    }     
    

    Just to be clear, we're hiding the Text property from the Readable class using the new keyword on the Text property in the Writeable class.
    From http://msdn.microsoft.com/en-us/library/ms173152(VS.80).aspx: When the new keyword is used, the new class members are called instead of the base class members that have been replaced. Those base class members are called hidden members. Hidden class members can still be called if an instance of the derived class is cast to an instance of the base class.