Applying Composition Over Inheritance to Page objects

Question

I'm trying to figure out how to apply composition over inheritance to a system I'm building, and tripping up when I try to apply it to my page objects.

The relevant objects are:

• PageFactory
• Page
• Route
• [Entity]Repository (multiple)

The system is pretty straight-forward. The PageFactory is used to build and retrieve Page objects. Pages use repositories to retrieve data objects for use when rendering the page and setting the page's title.

Before trying to apply composition over inheritance, each different page would be a subclass of the base Page object, and would override getData and getTitle functions.

I've refactored the system such that there are no longer subpage objects, but instead PageFactory creates an instance of Page for each type of page and sets the appropriate properties through setter functions (think view template path, route string, etc).

The problem is that each page needs to be able to retrieve data specific to that page using entity ids retrieved via route tags (like /entity/284), and also set the page's title based on the retrieved data, too.

This behavior is not easily refactored to config data. This is actual code that differs between pages.

The first approach I tried was to create an interface called iDataProvider and then in the PageFactory I would give each Page instance the appropriate concrete [entity]DataProvider object which would then have its own getData and getTitle functions. Page objects would retrieve all query data and pass them to the iDataProvider object when requesting data and title.

It works, but it feels icky, and I'm not 100% why. I think my feeling of unease may have something to do with passing all query data to the data provider and then asking it to know how to extract the important bits.

The second approach I tried was to have a single RouteDataProvider object which accepts a Route object, uses the Route object to get all query data, and then based on what query tags are in that route retrieves the appropriate entities and passes them back to the page. This really doesn't feel much better.

I feel like I'm designing myself into a corner. Where did I make the wrong turn, and what would be a better way to design this part of my system?

Answer 1

As far as I remember the principle states prefer composition over inheritance. Not "never use inheritance". Furthermore I think the original motivation was that pieces of duplicated code should be extracted into helper classes not in some sort of base class which in such case would be a permature abstraction. On the contrary, in your case Page is example of well defined abstraction. As a rule of thumb when you use "is-a" in your natural language it's an example of good abstraction. I.e. UsersPage is a Page. Also the idea that the page should construct itself is not that bad according to GRASP information expert principle. My conclusion is the same as Robert Harvey's: your initial design was pretty fine.

Answer 2

Creating a base Page class is a good idea. A "page" is a well known abstraction. The problem is the getData method. The data for any given page is going to be unique, so using a loosily typed language, or a strongly typed language that supports generics or class templates would be the right approach.

The data providers should be constructor arguments passed when initializing the concrete classes. It is not something the parent Page class should be concerned with.

The PageFactory probably should accept a route object. Data from the route can then be passed in to the new page object as explicit constructor arguments, or pass in the entire route object to the new page, along with the proper data provider. The route object could be passed up to the Page class's constructor while the data provider is used in the concrete child class.

Answer 3

Inheritance is not bad per se, however it is often misunderstood. I'm quoting some relevant parts of your question:

Before trying to apply composition over inheritance, each different page would be a subclass of the base Page object, and would override getData and getTitle functions. ... This behavior is not easily refactored to config data. This is actual code that differs between pages.

If the only methods in your Page class are getters, then you don't have any real behaviour. Subclassing to override getters isn't a good use of inheritance, and provides no benefit over passing the data to the constructor.

Instead, I'd look into making PageFactory an interface and having different implementations to create different kinds of pages (note that using interfaces is a form of inheritance too, but now there is actual behaviour involved):

interface PageFactory {
  boolean canHandle(Route route);
  Page createPage(Route route);
}

Then when you get a request, you can iterate through a list of page factories and use the one that can handle the route. This is pretty close to how many MVC frameworks work (PageFactory and Page roughly map to controller and model).