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?

• Robert Harvey: Where you made the wrong turn is you decided to favor composition over inheritance without considering the tradeoffs. Inheritance is not a dirty word; it is a tool, and like any other tool, it has its appropriate uses.
• Robert Bräutigam: The problems start with pulling data out of objects. As soon as you do that, now you are responsible for knowing what to do with it, hence the need to start sharing knowledge all over the place. Unfortunately you don't have enough details for me to draft some design. But, you should start with what you need these objects to do, and build from there. Do you need a Page produce a HTML page? A JSON object? Start with that, try to work your way backwards.

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.

• Robert Bräutigam: I disagree with your very first sentence. Having a "base" page in the sense we are talking about is exactly the reason inheritance has such a bad reputation. It is a misuse of inheritance. It is a workaround for "sharing" similar code between classes. The better solution being creating the necessary abstraction instead of just sharing random code blocks through inheritance.

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).

• Nathan Arthur: Clarification: getData and getTitle were protected methods that existed so that the base class could get the data needed to do the logic held in the base class.
• casablanca: @NathanArthur: You can pass title and data through the constructor and still have the same logic in the base class (without subclasses). My point was that inheritance should be used to override/extend behaviour, not to supply data.