Refactoring Inheritance to Composition

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

georgeoc
Forum Contributor
Posts: 166
Joined: Wed Aug 09, 2006 4:21 pm
Location: London, UK

Re: Refactoring Inheritance to Composition

Post by georgeoc »

OK then. Speaking purely in terms of class inheritance, we agree that I have far too much in the current design. But if I follow your suggestion of a base object (arborint's importers and exporters, or Keiran's message object) which has different 'flavours' depending on the external message format (and I fully agree that's what I should do), do I add extra functionality to the base class using inheritance or composition? Simply put, does the phpBB class inherit from the base class, or is it a separate object passed in to the base class to add functionality?

I wish I could put my question in clearer terms as regards OOP and Design Patterns terminology, but I just can't!

As a side note, I cannot manage to understand the thinking behind your 'message object', Keiran. To say I need "a message object which imports and exports" is like saying the 'message' object in a Blog application should perform the posting of itself. If messages are the 'currency' of my application, surely they need to be created, manipulated and exported by another class, and it's that one that I should be extending for different flavours of channel. Or am I misunderstanding you?
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Refactoring Inheritance to Composition

Post by Christopher »

I would extend a base class if the functionality is unique to that importer/exporter. However if you build your importers/exporters using functionality from various general purpose objects, such as different filters or translators, that might be used by several importers/exporters -- then I would composite those.

So both. ;)
(#10850)
User avatar
Kieran Huggins
DevNet Master
Posts: 3635
Joined: Wed Dec 06, 2006 4:14 pm
Location: Toronto, Canada
Contact:

Re: Refactoring Inheritance to Composition

Post by Kieran Huggins »

I think you get the general idea I was going for. The "base message" class is merely responsible for defining a standard data structure, as well as some basic normalizing (removing markup, etc) for said properties.

The specific "flavour" classes simply extend the base message, adding the import and export functions for that particular message format.

You'll always be instantiating a flavour class (never the base class) - once for importing the data from one format, and another for exporting the data to another format. The common base class gives them a standard data storage format and possibly some common methods for cleaning the data (as I mentioned above).

With that in mind - take another look at the code I posted earlier and see if it makes more sense.
georgeoc
Forum Contributor
Posts: 166
Joined: Wed Aug 09, 2006 4:21 pm
Location: London, UK

Re: Refactoring Inheritance to Composition

Post by georgeoc »

Thanks for your replies everyone. I've been away with work so haven't had much time to think about this.

I'd like to start my refactoring by removing the top-level inheritance from the channels - the dependence on the super-parent, m2f_object. Currently, each channel comes with an accompanying XML file. This specifies the fields the channel needs in order to operate (the pop3 class, for example, needs username, password, server, port, a boolean for APOP, etc). Using the XML file as a basis, my installation class creates a table in the database with those fields. When channels are saved in the future, the configuration for each is saved in a row in its own database table. The ORM is achieved using a mapper class. This queries the channel for the member variables that correspond to the fields in the table. The m2f_object parent class knows which fields are required for each object (also obtained and cached from the XML file), and passes an array of all the necessary variables to the mapper to create/update the table row.

Code: Select all

$pop = new m2f_pop;
$pop->set('username', 'testuser');
$pop->set('password', 'testpass');
$pop->set('server', 'localhost');
// ...etc
 
$mapper->save($pop); 
 
// $mapper queries $pop for all the fields necessary to make a new row in the database
$config_array = $pop->get_config();
// $mapper saves row in database
$db->save($config_array); // simplified!
 
If I am to change this inheritance to composition, I need to find a way to do it. Channels need accessors and mutators so they can be loaded from, and saved to the database by the mapper. Either the mapper or the channel needs to know which of the channel fields are specified in the XML configuration file (the POP3 channel, for example, may have further private fields that are not to be saved in the database). This all suggests to me that the parent class m2f_channel needs set() and get() functions, and that the mapper needs to know which fields it requires from each object.

So the mapper class does slightly more work to get the config from a channel:

Code: Select all

$mapper->save($pop); 
 
// mapper knows which fields it needs from $pop
$required_fields = array('username', 'password', 'server')
// $mapper queries $pop for all the fields necessary to make a new row in the database
$config_array = array();
foreach ($required_fields as $field)
{
   $config_array[$field] = $pop->get($field);
}
// $mapper saves row in database
$db->save($config_array); // simplified!
 
Does this sound like a good first step? Basically, I'm trimming down the channel objects by removing knowledge of how to interface with the database. Instead, I pass that knowledge to the mapper. This leaves channels needing nothing more than simple set() and get() methods.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Refactoring Inheritance to Composition

Post by Christopher »

It seems like either the pop object or the mapper should know the required fields and provide that information to the other.
(#10850)
Post Reply