|
Table of ContentsIntroductionThe TaskThe RequestThe DatabaseThe Code ExamplesThe Bad CodeWhy It's BadThe Good CodeThe Breakdown Of Why It's BetterIntroductionIt 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 TaskWhen we receive the request to process a single item from the queue we must add the capability to:
The RequestThe 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 DatabaseA database consisting of 3 tables:
However, the field names used by each table is different! Just to add to the complication. The Code ExamplesAll 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 CodeThe 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 BadThere are many but the main reasons I can think of right now are:
The Good CodeThe 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 & InterfacesGenerally 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 CodeGenerally 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 BetterNow 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).
Assigned TagsLanguages: 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 FollowingComments
there are 1 subscribed coders |