Refactoring Inheritance to Composition
Posted: Sun Jan 13, 2008 9:06 am
Hi all,
Before my copy of Martin Fowler's Refactoring: Improving the Design of Existing Code arrives next week, I would love to get some advice for the application I'm working on.
I have been getting more and more despondent with the development of my large-scale application, as I realise that my approach to OOP leaves quite a lot to be desired. It is my first real experience in both OOP and TDD, and although I feel I'm doing quite well (I have a smooth-running alpha release at least), I am now starting to see many areas which need improvement.
The major thing I've realised is that I've gone overboard with Inheritance, and made very little use of the power of Composition. I have read many similar stories, and don't particularly want to debate the relative merits in this thread. I can identify a number of cases where Composition seems more logical. I'm sure the Fowler book will help me get started, but I'd like some advice on how to choose between the two options, and how to rescue my application design (whose inheritance tree is extensive and highly coupled at the moment).
For your information, I'm currently developing for PHP 4 & 5 in one package, although I'm considering dropping PHP 4 support which will make my refactoring much easier and logical for all the OOP improvements of PHP 5.
First, some symptoms I've noticed:
Let me give an example. The application is a modular message centre. We use a proprietary message format internally (known as a Generic Message, or GM); the plugin modules are responsible for converting messages to and from that format. For example, it's possible to set up a chain between a POP3 import and a phpBB 3 export. The chain instructs the POP3 channel to import messages (from a mailbox) and convert them to GM format, then passes the GM objects to the phpBB channel which converts them into phpBB post format and exports them to a phpBB board.
The POP3 channel has this class name: m2f_channel_import_fetch_email_pop. This follows my current naming convention, and tells us that it inherits from:
So if we take the example of the m2f_channel_import_fetch_email_pop class. It does two distinct things - it must log in to a POP3 mailbox and retrieve a number of messages (fairly obvious). But then it needs to convert those messages into GM format. The POP3 functionality obviously belongs only in that class, but converting emails to GM needs to be at a lower level. Currently, when a POP object has retrieved some emails, it runs:
which runs the private method of the parent class, m2f_channel_import_fetch_email. But this makes the _transform() method difficult to test. Much better would be to have a separate object that is responsible for converting emails into GM, and have that object instantiated by the email channel, or passed in as a constructor parameter.
Having decided that, however, I'm now not sure how much inheritance I need at all. Do channels need to be arranged in a tree of functionality like they are currently (another example is m2f_channel_export_forum_phpbb3), or instead would it be better to have a simple 'channel' object with functionality like 'POP3' passed in to it via composition?
Sorry for the very long-winded post. It may make things easier if you scan over the files in my SVN repository: http://m2f.svn.sourceforge.net/viewvc/m2f/m2f/trunk/ - have a look at the /modules/channel directory to see what I've been talking about above.
I'm basically looking for general advice on how to spot unnecessary and wasteful Inheritance, and begin the job of using Composition to achieve the same ends. And more specific advice on whether I need a family-tree of classes for my channel modules, or whether a different approach may be better.
Thank you for any advice you can give!
(P.S. - I'm patiently waiting for the Search to be reindexed on the new phpBB 3 forum so I can do some more of my own research!)
Before my copy of Martin Fowler's Refactoring: Improving the Design of Existing Code arrives next week, I would love to get some advice for the application I'm working on.
I have been getting more and more despondent with the development of my large-scale application, as I realise that my approach to OOP leaves quite a lot to be desired. It is my first real experience in both OOP and TDD, and although I feel I'm doing quite well (I have a smooth-running alpha release at least), I am now starting to see many areas which need improvement.
The major thing I've realised is that I've gone overboard with Inheritance, and made very little use of the power of Composition. I have read many similar stories, and don't particularly want to debate the relative merits in this thread. I can identify a number of cases where Composition seems more logical. I'm sure the Fowler book will help me get started, but I'd like some advice on how to choose between the two options, and how to rescue my application design (whose inheritance tree is extensive and highly coupled at the moment).
For your information, I'm currently developing for PHP 4 & 5 in one package, although I'm considering dropping PHP 4 support which will make my refactoring much easier and logical for all the OOP improvements of PHP 5.
First, some symptoms I've noticed:
- I'm finding classes are becomming harder and harder to write tests for
- Important functionality often happens in private methods of parent objects, so is not testable (I currently try to suggest pseudo-private methods and properties in PHP 4 using an underscore prefix)
- Child objects are becomming bloated with unnecessary properties and functionality - even a simple Message object with subject, body and author field is very large because of all the unnecessary features it inherits.
- Although I hate the idea, I've come close to having a super-object to make ORM easier - every object that may need to be saved in the database descends from a class designed to be manipulated into a table row. This was an early mistake, and I plan to solve it with a simple Mapper which analyses an object and creates a database query.
Let me give an example. The application is a modular message centre. We use a proprietary message format internally (known as a Generic Message, or GM); the plugin modules are responsible for converting messages to and from that format. For example, it's possible to set up a chain between a POP3 import and a phpBB 3 export. The chain instructs the POP3 channel to import messages (from a mailbox) and convert them to GM format, then passes the GM objects to the phpBB channel which converts them into phpBB post format and exports them to a phpBB board.
The POP3 channel has this class name: m2f_channel_import_fetch_email_pop. This follows my current naming convention, and tells us that it inherits from:
- m2f_channel_import_fetch_email (a generic class to deal with imported email from any source - POP, mbox, IMAP, stdin),
- m2f_channel_import_fetch (an import channel which fetches messages on command, rather than listening for trigger messages),
- m2f_channel_import (a version of the channel class designed to import messages)
- m2f_channel (base channel class)
- m2f_element (an object designed to be part of a chain - other possibilities include filters and routers to transform messages)
- ... which descends from m2f_module (an object designed to be modular - drop files into the modules directory and they can be used instantly)
- ... which descends from m2f_object (the aforementioned object designed to be manipulated for entry into a database)
So if we take the example of the m2f_channel_import_fetch_email_pop class. It does two distinct things - it must log in to a POP3 mailbox and retrieve a number of messages (fairly obvious). But then it needs to convert those messages into GM format. The POP3 functionality obviously belongs only in that class, but converting emails to GM needs to be at a lower level. Currently, when a POP object has retrieved some emails, it runs:
Code: Select all
$this->_transform($messages)Having decided that, however, I'm now not sure how much inheritance I need at all. Do channels need to be arranged in a tree of functionality like they are currently (another example is m2f_channel_export_forum_phpbb3), or instead would it be better to have a simple 'channel' object with functionality like 'POP3' passed in to it via composition?
Sorry for the very long-winded post. It may make things easier if you scan over the files in my SVN repository: http://m2f.svn.sourceforge.net/viewvc/m2f/m2f/trunk/ - have a look at the /modules/channel directory to see what I've been talking about above.
I'm basically looking for general advice on how to spot unnecessary and wasteful Inheritance, and begin the job of using Composition to achieve the same ends. And more specific advice on whether I need a family-tree of classes for my channel modules, or whether a different approach may be better.
Thank you for any advice you can give!
(P.S. - I'm patiently waiting for the Search to be reindexed on the new phpBB 3 forum so I can do some more of my own research!)