5

One of the devs that works with me is following the Open/Closed Principle to add functionality to a Model by extending from our framework ORM.

class BaseModel extends ORM {
   ...
}

All our models are now inheriting BaseModel, and this model works well in the context of our application.

class Product extends BaseModel {
  ...
}

The problem comes when we tried to use the models as if they where extending directly from the ORM, use some of the most basic functionality:

$product = Product::factory();
$product->name = 'foo';
$product->price = 9.99;
$product->save();

This code should have worked well but it failed because now the factory method requires a list of parameters.

I think that as long as the models works well within the context of our product we are good; but I also have mixed feelings about not supporting some of the basic core functionality of the parent class.

Thoughts?

p.s.w.g
  • 4,135
  • 4
  • 28
  • 40
Onema
  • 185
  • 4
  • 3
    Is the Product class hiding the BaseModel::factory method? Has your coworker seen this: http://en.wikipedia.org/wiki/Liskov_substitution_principle ? – Steven Evers Apr 04 '13 at 16:27
  • See also http://stackoverflow.com/tags/lsp/info – Brian Apr 04 '13 at 16:31
  • 4
    All functionality inherited by the base class should work as it should be expected. That's part of the principle of least surprise and important for integration with other components which are designed for working with the base class. – Philipp Apr 04 '13 at 16:32
  • 2
    [What can go wrong if the Liskov substitution principle is violated?](http://programmers.stackexchange.com/questions/170222/what-can-go-wrong-if-the-liskov-substitution-principle-is-violated) & [How to verify the Liskov substitution principle in an inheritance hierarchy?](http://programmers.stackexchange.com/questions/170189/how-to-verify-the-liskov-substitution-principle-in-an-inheritance-hierarchy?lq=1) – Songo Apr 04 '13 at 17:25
  • 1
    All SOLID principles must be applied at the same time. You simply cannot pick one and use only that. And L is for Liskov Subsitution Principle. – Euphoric Apr 05 '13 at 16:09

4 Answers4

4

Any public accessible method is part of a contract on how things can be used. By requiring parameters on a descendent class' method, this contract is violated (see the already mentiond LSP).

Nevertheless, sometimes it is necessary to add parameters. In such case, the overriding method should still be able to use the original signature. In your example, this can be achieved by assigning reasonable default values to the parameters in BaseModel::factory().

nibra
  • 688
  • 3
  • 10
  • 1
    But Product::factory() is a static method, not an instance method so is not part of the contract for subtypes. (unless using something very odd like smalltalk) – Ian Jan 25 '16 at 13:23
2

In general, this really isn't a fundamental issue. Making child classes override functionality from their parents is a common and often-desirable trait of inheritance. It is needed especially if you want to do anything polymorphic.

But this case is a little different -- this is an issue of changing the inheritance chain of your classes. So really, it's the changes to the parent (BaseModel) that are the issue.

It sounds like your BaseModel class has changed (overridden) the static factory() method by adding additional required parameters. Assuming there is some good reason for the new parameter list, I think this is an OK change. It's just that these types of changes can have a real ripple effect to downstream classes.

But here's the real question: why are you relying on Product to inherit a static method? That seems a little strange to me, but it also allows for a potential fix. Just override factory() in your Product class so it doesn't require parameters, if possible. It can then call other factory() methods of the parent classes as needed.

Allan
  • 1,939
  • 12
  • 9
1

As nibra already stated, you have a contract for this public accessible method and you're not allowed to break it.

A really simple rule is, for an extended class, you can extend the type of input you accept, but be more specific on the output, so you don't break the contract.

Tobias
  • 728
  • 3
  • 8
1

The Liskov substitution principle restricts the behavior of instance methods in subclasses. It does not say anything about class methods like Product::factory.

kevin cline
  • 33,608
  • 3
  • 71
  • 142