How to apply the principle of open closing when creating objects

Question

I'm busy parsing XML documents (google docs api) and putting individual documents into objects.

There are various types of documents (documents, spreadsheets, presentations). Most of the information about these documents is the same, but some are different.

The idea was to create a base document class that contains all the general information, using subclasses for each specific type of document.

The problem is creating the correct classes for the different types. There are two ways to distinguish a document type. Each entry has a category item where I can find the type. Another method to be used is resourceId, in the form of type:id .

The most naive option would be to create an if statement (or switch-statement) that checks the type of record and create an appropriate object for it. But this will require editing the code if a new type is added.

Now I'm not quite sure if there is another way to solve this problem, so I ask about it here. I could encapsulate the creation of the desired object type in the factory method, so the required number of changes is minimal.

I now have something like this:

 public static function factory(SimpleXMLElement $element) { $element->registerXPathNamespace("d", "http://www.w3.org/2005/Atom"); $category = $element->xpath("d:category[@scheme='http://schemas.google.com/g/2005#kind']"); if($category[0]['label'] == "spreadsheet") { return new Model_Google_Spreadsheet($element); } else { return new Model_Google_Base($element); } } 

So my question is: is there another method that I do not see to deal with this situation?

Edit: Added sample code

Answer 1

Updated answer with sample code

Here is your new factory:

 public static function factory(SimpleXMLElement $element) { $element->registerXPathNamespace("d", "http://www.w3.org/2005/Atom"); $category = $element->xpath("d:category[@scheme='http://schemas.google.com/g/2005#kind']"); $className = 'Model_Google_ '.$category[0]['label']; if (class_exists($className)){ return new $className($element); } else { throw new Exception('Cannot handle '.$category[0]['label']); } } 

I'm not sure that I have exactly your point ... To rephrase the question, I realized: "How can I create the correct object without hard-coding the selection in my client code"

With autoload

So, let's start with the client base code.

 class BaseFactory { public function createForType($pInformations) { switch ($pInformations['TypeOrWhatsoEver']) { case 'Type1': return $this->_createType1($pInformations); case 'Type2': return $this->_createType2($pInformations); default : throw new Exception('Cannot handle this !'); } } } 

Now let's see if we can change this to avoid if / switch statuses (not always necessary, but maybe)

We will use the capabilities of PHP Autoload.

First, let's take a look at startup on site, here is our new Factory

 class BaseFactory { public function createForType($pInformations) { $handlerClassName = 'GoogleDocHandler'.$pInformations['TypeOrWhatsoEver']; if (class_exists($handlerClassName)){ //class_exists will trigger the _autoload $handler = new $handlerClassName(); if ($handler instanceof InterfaceForHandlers){ $handler->configure($pInformations); return $handler; } else { throw new Exception('Handlers should implements InterfaceForHandlers'); } } else { throw new Exception('No Handlers for '.$pInformations['TypeOrWhatsoEver']); } } } 

Now we have to add autoload capability

 class BaseFactory { public static function autoload($className) { $path = self::BASEPATH. $className.'.php'; if (file_exists($path){ include($path); } } } 

And you just need to register your autoloader, for example

 spl_autoload_register(array('BaseFactory', 'autoload')); 

Now, every time you have to write a new handler for types, it will be automatically added.

With chain of responsibility

You may not want to write something more “dynamic” in your factory, with a subclass that processes more than one type.

eg,

 class BaseClass { public function handles($type); } class TypeAClass extends BaseClass { public function handles($type){ return $type === 'Type1'; } } //.... 

In BaseFactory code, you can load all your handlers and do something like

 class BaseFactory { public function create($pInformations) { $directories = new \RegexIterator( new \RecursiveIteratorIterator( new \RecursiveDirectoryIterator(self::BasePath) ), '/^.*\.php$/i' ); foreach ($directories as $file){ require_once($fileName->getPathName()); $handler = $this->_createHandler($file);//gets the classname and create it if ($handler->handles($pInformations['type'])){ return $handler; } } throw new Exception('No Handlers for '.$pInformations['TypeOrWhatsoEver']); } } 

Answer 2

I agree with Oktopus, usually there are two methods without hardcoding; the first would be to find the class name by dynamically adding strings and make sure your classes are correctly named, the second way is to load all the handler classes and use the one that says it can handle this type. I would go first. With your code example, it looks something like this:

 <?php public static function factory(SimpleXMLElement $element) { $element->registerXPathNamespace("d", "http://www.w3.org/2005/Atom"); $category = $element->xpath("d:category[@scheme='http://schemas.google.com/g/2005#kind']"); $classname = sprintf( 'Model_Google_%s', ucfirst( strtolower( (string) $category[0]['label'] ) ) ); if( class_exists( $classname, true /* true means, do autoloading here */ ) ) { return new $classname( $element ); } return new Model_Google_Base($element); } 

About the chain of responsibility: although this is a great example of how to write it, I believe that the chain of responsibility means that all possible handlers should be loaded and asked if they can handle this type. The resulting code is cleaner and more explicit from my point of view, but there is both performance and complexity. That's why I went for the dynamic resolution of class names.

Answer 3

Perhaps you can apply xsl-tranformation to your input file (in the factory method). The result of the conversion should be a uniform xml file giving input on which class to use?

I know this is not much better than a lot of if , but in this way you at least avoid rewriting your code.