TravisSwicegood.com

11 October

Why inheritance sucks: part deux

In the comments of my previous post - Why class inheritance sucks - I got a comment that shows some of the confusion with composition versus inheritance in the world of objects. Sandy said:

...I have cases where, say, if I'm building a CMS, I have a ContentObject class that has generalized methods to determine where it's been published, etc. If I have two types of content, Document and Event, I fail to see why extending ContentObject for each one is bad.

Now if I have another type, Author, and each Document needs an Author, it's a has-a relationship. Extending Author to get the author's name as part of the Document would be wrong. Instead it should be $document->author->getName();

But in the first case, if I need to implement a publishTo($page_id) method, I'd rather do it once in ContentObject, so I can change that implementation in one place. Am I missing something that makes that a bad idea?

That is a pretty standard case where it looks like the best route is Event extends ContentObject, especially with a publishTo() method that seems so universal. Given any number of factors, this might actually be the best route for any given project. I approach OOP from a flexibility standpoint with one of its biggest flex-points being loose coupling which in turn promotes reusability. I'm going to use Event as a hypothetical example.

In the normal case, you might want to add it to a page of other events, making sure its ordered correctly, etc., etc. What happens when it needs to update Google Calendar, shoot off an SMS to the administrators, and publish to a page particular ContentObject page? If that happens, Event->publishTo($page_id) gets:

  • relegated to a call from some other method such as Event->publish()
  • gets repurposed to the point that publishTo($page_id) doesn't adequately convey what it's actually doing
  • or worse it gets ignored when someone new writes the publish() code because they weren't aware of the inherited method

Assuming we want avoid all of these, a composite in the simplest form would look something like this:

class Event {
  ...  logic and such ...

  public function publish() {
    $content_object = new ContentObject();
    $content_object->publish_data('some-page', $this);
    $google_cal = new GoogleCal();
    $google_cal->add_event($this);
    $sms = new SMSSender('admins@example.mobi');
    $sms->send_event($this);
  }
}

This gives us the composition and moves the logic of saving out to other objects, but isn't very flexible. Every new publishing type requires a change to Event::publish(). This is a place where the Visitor Pattern proves useful. The publish() method becomes:

public function publish(EventPublisher $publisher) {
  $publisher->publishEvent($this);
}

With that, you completely externalize the act of publishing an Event to any object that implements the EventPublisher interface. Now any object can be used for publishing an Event without any changes required to the actual Event code. Your commit messages will be shorter and less code will have to be written which should both translate into less of a likelihood for bugs to get introduced (not that they'd make it past that stellar TDD test suite you've coded ;-)). And you can still share your ContentObject::publishTo() functionality:

class ContentObject implements EventPublisher, DocumentPublisher
{
  public function publishTo($page_id) {
    ... do publishing ...
  }

  public function publishEvent(Event $event) {
    $this->name = $event->name;
    ... etc., etc. ...
    $this->publishTo('some-page');
  }

  public function publishDocument(Document $document) {
    $this->name = $document->filename;
    ...  etc., etc. ...
    $this->publishTo('some-page');
  }
}

The careful observer will notice that while ContentObjectuses the property $name in the above example, the property that Event and Document use is different. With the above functionality you don't end up having to shoe-horn property names into specific schemas because of inheritance allowing the best practice to dominate the code instead of the best choice.

And now the original "what if" problem. Publishing all of the code might look something this:

$publisher = new MultipleEventPublisher(array(
  new ContentObject('calendar-page'),
  new GoogleCal('my-events'),
  new EventSMSPublisher('8885554321@example.mobi')
));
$event->publish($publisher);

Now instead of Event being a ContentObject, it uses one. The shift paradigm is subtle, but something that can help produce better, more flexible code.

5 comments

Thank you for interesting post.

One note:
Using Observer pattern is another approach that may be useful here. For example, Content object may have Publishing event and different CMS modules may attach listeners to that event and perform actual publishing.

I also have one interesting example when inheritance is not applicable. Consider three classes and relations between them: point, circle and ellipse. Is is correct to extends ellipse from circle?

In response to previous comment: the case with circle and ellipse is the border example. In mathematics, a circle is an ellipse, while in OOP, this is not so, because they implement different properties and pre/post-conditions.

For example: there is a rule for ellipse that can be expressed as a unit test: if you change the x-axis, the y-axis remains unchanged. If a programmer develops a method that operates on a Eclipse, then he rightfully expects such behaviour. If you'd pass a Circle (which extends Ellipse), the expected behaviour breaks the Eclipse-rule.

So, Circle shouldn't extend Ellipse, even though the geometric equation for a circle is a simplified equation for an ellipse.
Sometimes my inherited objects end up being nests of bugs. I don't have a good example handy, but imagine overriding a few methods to provide some extra functionality, and simply inheriting others. Now the definition for the subclass is scattered across two classes (and two files). Also, the control jumps between the two classes a lot, so in the end, it's virtually impossible to follow. So much for all that work being saved through the magic of Inheritance.
Alex, fett covered the Circle/Eclipse issue pretty thoroughly, so I'll just add that your extension should be more of a Circle extends Shape, Eclipse extends Shape, if in fact there is a need for sub-classing at all.

The Observer I'll touch on though as it's an interesting pattern with a pretty tough call on when to use it. Observers and Visitors are actually very, very close in implementation, with one exception. The Observer implies that it needs updates on every change of the state of an object, while the Visitor only wants to be invoked on occasion. In the example above, publishing data, that qualifies as a do once event. To be true to the Observer pattern, every time the Event object's location changed, the registered Observer should be notified so it can determine whether it wants to do anything with the current state.

Assuming the ability to work with an Event at each step is desired, the Observer might be the better choice, but in the example above I would stick with the Visitor pattern as it answers a much more specific problem. I'll also venture a guess that a lot of the implementations of the Observer pattern would probably be better stated had they been expressed via a Visitor.

sapphirecat: yup... :-) What makes it even more fun is when you override the method to change functionality so now publishTo() doesn't even call the original publishTo() at all.
Observer is a good pattern to use in a stateful system. The classic example is in a GUI written using true-MVC (not the bastardized MVC2 popularized by Rails frameworks), where the View is a continually running stateful object and observes a continually running stateful Model object, and updates itself whenever necessary. It observes that there is a change in the model and updates itself accordingly.

In a shared-nothing, stateless system (like any PHP code), Observer is not a great solution because of the overhead. Setting up all of those observer relationships takes time and complexity, and generally you'll only ever need one or two in a given run of the script. That's a lot of wasted setup for little return. An "active" approach (Visitor, Strategy, etc.) works better there than a passive approach (Observer), because it requires less up-front setup.

Comments are closed for this post.