Page 2 of 2
Re: Refactoring Inheritance to Composition
Posted: Thu Jan 17, 2008 4:48 pm
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?
Re: Refactoring Inheritance to Composition
Posted: Thu Jan 17, 2008 5:02 pm
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.

Re: Refactoring Inheritance to Composition
Posted: Thu Jan 17, 2008 5:20 pm
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.
Re: Refactoring Inheritance to Composition
Posted: Wed Jan 23, 2008 1:46 pm
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.
Re: Refactoring Inheritance to Composition
Posted: Wed Jan 23, 2008 2:26 pm
by Christopher
It seems like either the pop object or the mapper should know the required fields and provide that information to the other.