Scott Thompson

I have 22 followers

Introduction

It is all to often seen that when someone attempts to explain anything to do with Object Oriented Programming, they always tend to dumb it down using real life objects. For example, to show aggregation in this way, you will use an instance of a Person driving an instance of a Car. This correctly shows aggregation because both can survive without each other. However, this fails to show how you should use aggregation in real life systems (unless you are creating a game perhaps) and this is often where experience comes in to play. One that is often easier to explain using real life objects are interfaces, but much harder to show the practical use of them.

Throughout this blog post I am going to be creating a script that will process a "Content Moderation Queue" and show you both how not to do it, and then a much better way to do it using the Factory Pattern, Polymorphism and Interfaces.

This blog post will not teach you what interfaces are, what abstract classes are or the basics of Object Oriented Programming. It will instead attempt to teach you when and why you would use the Factory Pattern, Abstract Classes and Interfaces using PHP to show you via code samples.

The Task

When we receive the request to process a single item from the queue we must add the capability to:

  • Accept the item
  • Delete the item
  • Know how to handle specific "types" such as; Personal Messages, Comments and Forum Posts
  • Ability to add more "types" in the future
  • When a personal message is accepted, send a notification

The Request

The request to process a single item from the queue will be made via a $_GET request containing the type of content being moderated, the id of the item in the database and what our action is for example:

/ajax/content-moderation-queue/process?type=comment&id=23&action=accept

The Database

A database consisting of 3 tables:

  • personal_messages
  • forum_posts
  • comments

However, the field names used by each table is different! Just to add to the complication.

The Code Examples

All code shown is for demonstration purposes only and will not be fully functional code and please remember I am only trying to demonstrate the factory pattern and polymorphism usage - not the use of MVC and other design patterns and best practices that would often accompany such code. I have also used the die() function in replacement of correct error handling. However, the code will correctly show what I am trying to demonstrate for the sake of this post.

Upon execution of the code, it is assumed that the correct checks on the permissions of the user performing the action has already been done along with the connection to the database as well as correct data sanitisation to prevent SQL injection attacks. The use of the static query method db::query() is a wrapper that is assumed to perform the SQL query.

The Bad Code

The following code contains many smells that indicate that it needs to be refactored (more on those smells later). Also be aware that I have NOT tested this code so a few syntax errors and such may exist but I am sure you can use your existing knowledge of PHP to fill in those errors should you find any.

<?php

//===========================================================
//=== VALIDATION THAT THE PARAMS RECEIVED ARE ACCEPTABLE
//===========================================================

//ensure params are set
if (isset($_GET['type']) == false || isset($_GET['id']) == false || isset($_GET['action']) == false) {
	die("Error: Incorrect params.");
}

//clean the id
$_GET['id'] = intval($_GET['id']);

//ensure the type is valid (using select easily lets us add and remove valid types)
switch ($_GET['type']) {
	case 'personal_message':
	case 'forum_post': 
	case 'comment': 
		break;
	default: die("Error: Unknown type.");
}

//ensure the action is valid (using select easily lets us add and remove valid actions)
switch ($_GET['action']) {
	case 'accept':
	case 'delete': 
		break;
	default: die("Error: Unknown action.");
}

//===========================================================
//=== PROCESS THE ACTION
//===========================================================

//ACCEPT ACTION
if ($_GET['action'] == 'accept') {
	switch ($_GET['type']) {
		case 'personal_message': 
			//accept the content
			db::query("UPDATE 'personal_messages' SET 'moderation_required' = 'n' WHERE 'pm_id' = '{$_GET['id']}' LIMIT 1;");
			//send the notification to the member that they have a new message
			//{code here to send the notification}
			break;
		case 'forum_post': 
			//accept the content
			db::query("UPDATE 'forum_posts' SET 'moderation_required' = 'n' WHERE 'post_id' = '{$_GET['id']}' LIMIT 1;");
			break;
		case 'comment': 
			//accept the content
			db::query("UPDATE 'comments' SET 'moderation_required' = 'n' WHERE 'comment_id' = '{$_GET['id']}' LIMIT 1;");
			break;
	}
}

//DELETE ACTION
if ($_GET['action'] == 'delete') {
	switch ($_GET['type']) {
		case 'personal_message': 
			//delete the content
			db::query("DELETE FROM 'personal_messages' WHERE 'pm_id' = '{$_GET['id']}' LIMIT 1;");
			break;
		case 'forum_post': 
			//delete the content
			db::query("DELETE FROM 'forum_posts' WHERE 'post_id' = '{$_GET['id']}' LIMIT 1;");
			break;
		case 'comment': 
			//delete the content
			db::query("DELETE FROM 'comments' WHERE 'comment_id' = '{$_GET['id']}' LIMIT 1;");
			break;
	}
}

?>

Why It's Bad

There are many but the main reasons I can think of right now are:

  • It's tightly coupled to using the $_GET superglobal meaning it would be hard to remove the code and put it into another environment that doesn't use $_GET (such as the console) or to pass it params from elsewhere like a database.
  • The "code smell" I mentioned earlier is seeing the near identical switch conditions scattered throughout which also violates the DRY (Dont Repeat Yourself) principle and would mean that if you need to update one, you will need to remember to update the others as well! This is asking for trouble. Seeing all the switch conditions is often a good sign that using polymorphism would help.
  • Repeated common behaviour i.e. deleting items from the database and updating the moderation_required db field. This common behaviour should be grouped together and abstracted if possible to further reduce the code.
  • There is no clearly set method of adding new types of content other than going through all the code, changing all the switches to accept it, and putting the new code inline. The way in which the content is accepted should be hidden from the client using it. In this case, the client knows way to much about everything and a single change could impact a LOT of other areas in the code. This makes updates more risky in the future as well as the page possibly becoming unmanageable as more types of content are added and they contain more complex ways of accepting and deleting them.

The Good Code

The following code fixes all those bad points mentioned above and will explain how as I break it down in a minute. First of, the whole code. Also be aware that I have NOT tested this code so a few syntax errors and such may exist but I am sure you can use your existing knowledge of PHP to fill in those errors should you find any.

The Classes & Interfaces

Generally put into the framework you are using.

<?php

//===========================================================
//=== THE INTERFACES
//===========================================================

/**
* any object that implements this can be returned using the moderation_queue_factory and everything will still work as expected
*/
interface moderation_queue_processable {
	public function accept($id);
	public function delete($id);
}

//===========================================================
//=== THE CLASSES
//===========================================================

/**
* finds out what object to return that is capable of processing the specific type of content from the moderation queue
*/
class moderation_queue_factory {
	
	/**
	* prevent this class being instantiated
	*/
	private function __construct() { }
	
	/**
	* return the correct instance of the item to be handled
	* @param string $type passing the type like this no longer ties it to use the $_GET global
	* @return moderation_queue_processable|null we do not care what object is to be returned as long as it implements the moderation_queue_processable interface
	*/
	public static function get_instance($type) {
		switch ($type) {
			case 'personal_message': return new moderation_queue_item_personal_message();
			case 'forum_post': return new moderation_queue_item_forum_post();
			case 'comment': return new moderation_queue_item_comment();
			default: return null;
		}
	}
	
}

/**
* this defines the core attributes to be used in order to setup the sub classes
*/
abstract class moderation_queue_item implements moderation_queue_processable {
	
	/**
	* used to map specific field and table names to construct the correct query
	*/
	protected $db_mapping = array(
		'table_name' => '',
		'id_field_name' => '',
		'moderation_field_name' => ''
	);
	
	/**
	* accept the content (enforced by the interface)
	* @param int $id of the item to delete from the db
	*/
	public function accept($id) {
		db::query("UPDATE '{$this->db_mapping['table_name']}' SET '{$this->db_mapping['moderation_field_name']}' = 'n' WHERE '{$this->db_mapping['id_field_name']}' = '{$id}' LIMIT 1;");
	}
	
	/**
	* delete the content (enforced by the interface)
	* @param int $id of the item to delete from the db
	*/
	public function delete($id) {
		db::query("DELETE FROM '{$this->db_mapping['table_name']}' WHERE '{$this->db_mapping['id_field_name']}' = '{$id}' LIMIT 1;");
	}
	
}

/**
* knows how to process forum posts from the content moderation queue
*/
class moderation_queue_item_forum_post extends moderation_queue_item {

	/**
	* used to map specific field and table names to construct the correct query
	* @override
	*/
	protected $db_mapping = array(
		'table_name' => 'forum_posts',
		'id_field_name' => 'post_id',
		'moderation_field_name' => 'moderation_required'
	);
	
}

/**
* knows how to process comments from the content moderation queue
*/
class moderation_queue_item_comment extends moderation_queue_item {

	/**
	* used to map specific field and table names to construct the correct query
	* @override
	*/
	protected $db_mapping = array(
		'table_name' => 'comments',
		'id_field_name' => 'comment_id',
		'moderation_field_name' => 'moderation_required'
	);
	
}

/**
* knows how to process personal messages from the content moderation queue
*/
class moderation_queue_item_personal_message extends moderation_queue_item {

	/**
	* used to map specific field and table names to construct the correct query
	* @override
	*/
	protected $db_mapping = array(
		'table_name' => 'personal_messages',
		'id_field_name' => 'pm_id',
		'moderation_field_name' => 'moderation_required'
	);
	
	/**
	* Need to add extra functionality to sent a notification
	* @param int $id of the item to accept
	* @override
	*/
	public function accept($id) {
		
		//accept it as normal
		parent::accept($id);
		
		//also send a notification
		//{code here to send the notification}
		
	}
	
}

The Client Code

Generally put into the ajax php file (or the modal if using MVC - minus the messages returned by using die()). Notice how much shorter and easier to read this is compared to the original "bad" code. In fact the client using the object may not even no or care what object it's using due to using a factory and polymorphism! All it knows is the accept() and delete() methods exist and that's all it needs to know.

//===========================================================
//=== VALIDATION THAT THE PARAMS RECEIVED ARE ACCEPTABLE
//===========================================================

//ensure params are set
if (isset($_GET['type']) == false || isset($_GET['id']) == false || isset($_GET['action']) == false) {
	die("Error: Incorrect params.");
}

//===========================================================
//=== GET THE CORRECT OBJECT TO HANDLE THE CONTENT
//===========================================================

$obj_item_handler = moderation_queue_factory::get_instance($_GET['type']);
if ($obj_item_handler === null) {
	die("Error: Unknown type.");
}

//===========================================================
//=== PROCESS THE ACTION
//===========================================================

//ensure the action is valid (using select easily lets us add and remove valid actions)
switch ($_GET['action']) {
	case 'accept': $obj_item_handler->accept(intval($_GET['id']); break;
	case 'delete': $obj_item_handler->delete(intval($_GET['id']); break;
	default: die("Error: Unknown action.");
}

?>

The Breakdown Of Why It's Better

Now I will break down the code above to show and explain how Object Oriented Programming has helped simplify the code and aid maintenance.

Interfaces

/**
* any object that implements this can be returned using the moderation_queue_factory and everything will still work as expected
*/
interface moderation_queue_processable {
	public function accept($id);
	public function delete($id);
}

I have shown you how to use interfaces so that ANY object that implements "moderation_queue_processable" can be inserted into the factory method in the "moderation_queue_factory" class and it will work without any further modifications! Brilliant!

Factory Design Pattern

/**
* finds out what object to return that is capable of processing the specific type of content from the moderation queue
*/
class moderation_queue_factory {
	
	/**
	* prevent this class being instantiated
	*/
	private function __construct() { }
	
	/**
	* return the correct instance of the item to be handled
	* @param string $type passing the type like this no longer ties it to use the $_GET global
	* @return moderation_queue_processable|null we do not care what object is to be returned as long as it implements the moderation_queue_processable interface
	*/
	public static function get_instance($type) {
		switch ($type) {
			case 'personal_message': return new moderation_queue_item_personal_message();
			case 'forum_post': return new moderation_queue_item_forum_post();
			case 'comment': return new moderation_queue_item_comment();
			default: return null;
		}
	}
	
}

I have shown you how to remove the knowledge from the client about what type of object to create. Now, the factory has sole responsibility for creating the object of the specified type and it knows that it can return ANY object that implements the "moderation_queue_processable" interface. It does not care about anything else. This also encapsulates the object creation process making it very easy to add and remove types.

Now the process of allowing a new content type to be accepted or deleted is simply a matter of implementing the "moderation_queue_processable" interface and updating the factory method. Nothing else will need to be changed! Much less error prone, much more flexible and more easier to maintain.

Abstract Classes

/**
* this defines the core attributes to be used in order to setup the sub classes
*/
abstract class moderation_queue_item implements moderation_queue_processable {
	
	/**
	* used to map specific field and table names to construct the correct query
	*/
	protected $db_mapping = array(
		'table_name' => '',
		'id_field_name' => '',
		'moderation_field_name' => ''
	);
	
	/**
	* accept the content (enforced by the interface)
	* @param int $id of the item to delete from the db
	*/
	public function accept($id) {
		db::query("UPDATE '{$this->db_mapping['table_name']}' SET '{$this->db_mapping['moderation_field_name']}' = 'n' WHERE '{$this->db_mapping['id_field_name']}' = '{$id}' LIMIT 1;");
	}
	
	/**
	* delete the content (enforced by the interface)
	* @param int $id of the item to delete from the db
	*/
	public function delete($id) {
		db::query("DELETE FROM '{$this->db_mapping['table_name']}' WHERE '{$this->db_mapping['id_field_name']}' = '{$id}' LIMIT 1;");
	}
	
}

I have shown you how to abstract similar behaviour and pass that behaviour on to sub classes using inheritance. This reduces copy and pasting and results in less areas to change in the future aiding maintenance.

The reason this class is set as abstract is it has no context (basically it won't work) until it is extended and the $db_mapping property has the correct values entered. It already defines the default behaviour for accept and delete methods.

Poloymorphism

/**
* knows how to process forum posts from the content moderation queue
*/
class moderation_queue_item_forum_post extends moderation_queue_item {

	/**
	* used to map specific field and table names to construct the correct query
	* @override
	*/
	protected $db_mapping = array(
		'table_name' => 'forum_posts',
		'id_field_name' => 'post_id',
		'moderation_field_name' => 'moderation_required'
	);
	
}

/**
* knows how to process comments from the content moderation queue
*/
class moderation_queue_item_comment extends moderation_queue_item {

	/**
	* used to map specific field and table names to construct the correct query
	* @override
	*/
	protected $db_mapping = array(
		'table_name' => 'comments',
		'id_field_name' => 'comment_id',
		'moderation_field_name' => 'moderation_required'
	);
	
}

/**
* knows how to process personal messages from the content moderation queue
*/
class moderation_queue_item_personal_message extends moderation_queue_item {

	/**
	* used to map specific field and table names to construct the correct query
	* @override
	*/
	protected $db_mapping = array(
		'table_name' => 'personal_messages',
		'id_field_name' => 'pm_id',
		'moderation_field_name' => 'moderation_required'
	);
	
	/**
	* Need to add extra functionality to sent a notification
	* @param int $id of the item to accept
	* @override
	*/
	public function accept($id) {
		
		//accept it as normal
		parent::accept($id);
		
		//also send a notification
		//{code here to send the notification}
		
	}
	
}

I have shown you how to remove multiple switch conditions using Polymorphism (inheritance) and in doing so you are able to keep to the DRY (Don't Repeat Yourself) principle. Again, this further aids maintenance and reduces the possibility of creating bugs.

The Client

//===========================================================
//=== VALIDATION THAT THE PARAMS RECEIVED ARE ACCEPTABLE
//===========================================================

//ensure params are set
if (isset($_GET['type']) == false || isset($_GET['id']) == false || isset($_GET['action']) == false) {
	die("Error: Incorrect params.");
}

//===========================================================
//=== GET THE CORRECT OBJECT TO HANDLE THE CONTENT
//===========================================================

$obj_item_handler = moderation_queue_factory::get_instance($_GET['type']);
if ($obj_item_handler === null) {
	die("Error: Unknown type.");
}

//===========================================================
//=== PROCESS THE ACTION
//===========================================================

//ensure the action is valid (using select easily lets us add and remove valid actions)
switch ($_GET['action']) {
	case 'accept': $obj_item_handler->accept(intval($_GET['id']); break;
	case 'delete': $obj_item_handler->delete(intval($_GET['id']); break;
	default: die("Error: Unknown action.");
}

The client (code) requiring the functionality of processing the items from the queue has been drastically reduced, simplified and many responsibilities have been removed. This in turn makes it easier to allow content from the queue to be processed via different methods such as via the command line, ajax (currently), through POST requests, and so on... while ensuring you only have one copy of code doing the hard work (i.e. the classes) and sticking to the DRY principle (Don't Repeat Yourself).

2 0
please help rate the author by liking / disliking their work

Assigned Tags

Languages: Php

Technologies: Abstract Classes, Design Patterns, Factory Pattern, Inheritance, Interfaces, Object Oriented Programming, Polymorphism, Refactoring

Categories: Design Patterns, General Programming, Refactoring

More Blog Posts By VBAssassin

Blog Posts By Coders VBAssassin Is Following

Comments

You can see another perspective on these design patterns in this article: http://www.ibm.com/developerworks/opensource/library/os-php-designptrns/index.html

Posted 11 Days Ago by cmwosdude reply

I would also like to point out that in that link posted by cmwosdude that it also explains 4 other patterns I have not touched in this post that anyone learning about design patterns may find useful :) thanks for posting the link mate, but it is already in my web directory... Both that one and the next article covering 5 more patterns as they are good articles for beginners only it would help a lot more if more practicle examples were used. Still good articles though!

Posted 11 Days Ago by VBAssassin reply
Login to subscribe to comments
there are 1 subscribed coders
Please login to post comments or register for free