Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

Let's say we have this base class:

class MyBase
{
    public string Name { get; set; }

    public string GetDescription()
    {
        var descriptionList = new List<string>() { this.Name };
        descriptionList.AddRange(this.ExtraDescriptions.ToList());
        return string.Join(", ", descriptionList();
    }

    protected virtual IEnumerable<string> ExtraDescriptions()
    {
        return new List<string>();
    }
}

Then add a derived class:

class MyDerived : MyBase
{
    protected override IEnumerable<string> ExtraDescriptions()
    {
        return new List<string>() { "a", "b" };
    }
}

It seems to me that this violates the LSP (because we're changing the behavior of the GetDescription method), and yet it's also a fundamental feature of object-oriented programming. The base class obviously declares its intention to allow the ExtraDescriptions method to be overridden, but this isn't necessarily visible to a consumer of the class.

Ever since I learned about the LSP, this problem has really been gnawing at me. So:

  1. Does this violate LSP?
  2. What's the correct alternative to this?
share|improve this question
add comment

6 Answers

up vote 7 down vote accepted

LSP only requires that a property of a super-class be true of all its derivatives. By "property", in this context, we mean that "it will supply a list of descriptions". The implementation of that is irrelevant.

As far as I can see, there is no violation in your case.

Properties in LSP are much more about the assumptions that can be made by the calling code than they are about the implementation of functionality, or even behaviour.

The canonical example is "a square is a specialised rectangle." But rectangles have height and width properties, which calling code may assume are independent of each other. While a square only really has one dimension, so if I say rect.X = 3; rect.Y = 2; and the object is a rectangle, I get a 3x2 rectangle; but with the same calling code on a square, I get a 2x2 rectangle.

share|improve this answer
add comment

It is valid and called template method pattern. It is useful to reduce sequential coupling.

As an alternative, you can use closures, metaprogramming or patterns like policy.

see : http://sourcemaking.com/design_patterns/template_method for template method pattern and many others.

share|improve this answer
    
The template method pattern requires that the base class be declared abstract (and typically ExtraDescriptions would be declared abstract as well). In this particular case, I've made my base class concrete. –  Scott Whitlock Nov 17 '11 at 12:53
3  
Well this is just a template method pattern with a default behaviour provided. –  deadalnix Nov 17 '11 at 12:55
    
@ScottWhitlock, it is assumed but not required. The pattern may work perfectly with a concrete base class (which provides default implementations for the virtual methods representing the steps of the algorithm). –  Péter Török Nov 17 '11 at 12:57
    
@deadalnix - It sounds to me like the correct alternative is to define the Template Method in an abstract base class, then provide a default implementation of it in one class, and a different implementation in another class. That way my derived class doesn't inherit from the default implementation, so it doesn't change its base class' behavior. –  Scott Whitlock Nov 17 '11 at 12:58
1  
@ScottWhitlock > Your solution involve more complexity. You should have a rationnal behind this addition of complexity, other than the base class MUST be abstract. If it does make sense to provide a default implementation, then you just put it in the base class unless you have a reason not to. Making the base class abstract isn't a valide rason. Plus, you can declare your base class abstract even if you have no abstract methods in it in many languages if it doesn't make sense to instanciate it « as this ». –  deadalnix Nov 17 '11 at 13:02
add comment

I think it depends on what is the expected (published / documented) behaviour / contract of the base class / interface.

If GetDescription is meant to return some general, loosely defined description text (comma-separated list of words), it is perfectly fine for different derived classes to add different items to its output via overriding ExtraDescriptions. This is the Template Method pattern in action. [Update] In other words, LSP is not violated in this case.[/Update]

If OTOH it is declared to always return the name of the object and nothing else, using a Template Method to open up a possibility of breaking the interface contract wouldn't make the slightest sense. [Update] In other words, LSP would be violated in this case.[/Update]

share|improve this answer
    
As I commented on @deadalnix's answer, I believe for this to be the Template Method pattern, then ExtraDescriptions must be declared abstract, and then that certainly doesn't violate the LSP. It looks to me like the answer should be to create an abstract base class that implements the Template Method pattern, and then have a Default implementation that returns an empty list for ExtraDescriptions, and another implementation that returns other lists. –  Scott Whitlock Nov 17 '11 at 12:55
    
I can only second @deadalnix's response to your comment. IMO you are overcomplicating your design in search for some high ideal which usually turns out impractical in real life. The overhead doesn't seem big with a single abstract method - but just try to imagine how would your solution scale with ten abstract methods, of which various subclasses require the default behaviour of different ones? –  Péter Török Nov 17 '11 at 13:14
    
+1 - this answer is clearer, thanks! –  Scott Whitlock Nov 17 '11 at 14:08
add comment

The base class, not the derived class is responsible for LSP. So the creator of the base class should make his class open for substitution. This means that if the base class makes a method virtual, it should be overridable by any function without changing the functionality desirable properties of the class.

share|improve this answer
add comment

Ok, firstly, what you are doing doesn't necessarily violate LSP, if I understand the context correctly. The LSP reads:

Let q(x) be a property provable about objects x of type T. Then q(y) should be provable for objects y of type S where S is a subtype of T.

Now let's translate that to classes:

Let q(x) be a property of all instances x of class T. Then q(y) should also be a property of all instances y of any class S that is a subclass of T.

Now given the context, that q here is basically: a call to the object's GetDescription-method will return a meaningful description of that object.

In this case you're not violating it. On the contrary, one could argue that you're actually enforcing it, because if you did not override the method accordingly, then the result of GetDescription would in fact not be meaningful and thus LSP would not hold.
To contrast this, here's a real violation:

class MyUndescribable : MyBase
{
    override public string GetDescription()
    {
        throw "can't touch this!";
    }
}

I would argue that applying the LSP to non-public members is a bit pointless, because you making any assumptions about non-public members of an instance means violating encapsulation. The only time where this is worth considering is, when an instance of A calls a protected method of another instance of A (which could actually be an instance of a class B that has overridden this method).

Secondly, what you are doing would be better achieved through composition, following the DIP. The logic from ExtraDescriptions should be passed in, either as a function object, or as an abstract service. This way you could still derive MyDerived from MyBase (if you really wanted), and pass the logic to the super constructor for example. Thus MyDerived would be further decoupled from MyBase.

share|improve this answer
add comment

Based on @deadalnix and @PeterTorok's answers, here is, I think, a better implementation that doesn't violate LSP:

abstract class AbstractBase
{
    public string Name { get; set; }

    public string GetDescription()
    {
        var descriptionList = new List<string>() { this.Name };
        descriptionList.AddRange(this.ExtraDescriptions().ToList());
        return string.Join(", ", descriptionList();
    }

    protected abstract IEnumerable<string> ExtraDescriptions();
}

class DefaultImplementation : AbstractBase
{
    protected override IEnumerable<string> ExtraDescriptions()
    {
        return new List<string>() { };
    }
}

class OtherImplementation : AbstractBase
{
    protected override IEnumerable<string> ExtraDescriptions()
    {
        return new List<string>() { "a", "b" };
    }
}

I also think you could do what @deadalnix said in a comment, which is move the default implementation into the abstract class, like this:

abstract class AbstractBase
{
    public string Name { get; set; }

    public string GetDescription()
    {
        var descriptionList = new List<string>() { this.Name };
        descriptionList.AddRange(this.ExtraDescriptions().ToList());
        return string.Join(", ", descriptionList();
    }

    protected virtual IEnumerable<string> ExtraDescriptions()
    {
        return new List<string>() { };
    }
}

class DefaultImplementation : AbstractBase
{
}

class OtherImplementation : AbstractBase
{
    protected override IEnumerable<string> ExtraDescriptions()
    {
        return new List<string>() { "a", "b" };
    }
}

This allows any overriding class to re-use the default implementation.

share|improve this answer
    
This is what you want if it DOESN'T make sens to instanciate the base class. Neither in the question nor in this answer you give rationnale why you don't whant to instanciate this class. If you are in this case, this solution is valid, but otherwise, this is just adding complexity based on belief. It could be what you whant depending on the case you face. It has nothing to do with LSP. –  deadalnix Nov 17 '11 at 13:17
2  
Your second solution is almost the same as what we proposed. Making the base class abstract is fine - in fact, making it concrete is usually a bad idea for various reasons, with or without Template Method. However, if you do need a default implementation with precisely the same behaviour as the base class, your empty DefaultImplementation class is a useless complication. –  Péter Török Nov 17 '11 at 13:18
    
@deadalnix, Peter - I agree with what you're saying. But nobody's answered my question... "does this violate the LSP?" I think that, first of all, it does, and secondly, nobody really seems to care about it, so why is it such an oft-quoted principle? –  Scott Whitlock Nov 17 '11 at 13:29
1  
Neither of this is violating LSP as long as the overriden function behave as expected by someone using the base class or as expected in the base class. All are LSP compliant. The choice of one or another depends on other criterias. –  deadalnix Nov 17 '11 at 13:32
    
@ScottWhitlock, we attempted to answer your question indeed, sorry if it wasn't clear for you. I updated my answer to make it clearer. –  Péter Török Nov 17 '11 at 13:43
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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