redbullmarky Posted December 9, 2007 Share Posted December 9, 2007 Hi all I've been doing a bit of reading up on a few of the simpler patterns recently, especially those that handle object creation/storage for ease of access/use across my system. Not just any old objects, but moreso for those that are sort of global anyway - session, request (wrapper class for accessing GPC stuff), DB and config. So my question - are there any serious pitfalls (putting best-practice aside for a bit) of combining the registry with a factory, so i'd have factory methods such as getSession/getRequest (all under one factory roof), etc but where they actually store the instance for the next time they're required? Looking under the bonnet of Joomla, they do something sorta like this. From what I've gathered/understood so far from other sources, the normal Factory returns a new instance of a requested object, rather than a previously instantiated one. Other implementations have each class using their own factory - so instead of Factory::getSession(), it'd be more like Session::getInstance() - though wouldn't the latter mean that I've have lots of Singletons running all over the place rather than just the one? Is the way I explained still classed as the Factory pattern and i've just got my understanding muddled? Or is it really a dirty love child of both patterns? Any thoughts? Cheers Mark Quote Link to comment Share on other sites More sharing options...
koen Posted December 9, 2007 Share Posted December 9, 2007 I've also learned from the way joomla did things and their take on the registry is something I like. Previously I have singletons all over the place, now I use a factory as a central point for getting singletons and object creation logic (like a 'real' factory). I haven't run into any unwanted side effects yet. But I have to admit I sometimes also have the feeling I'm overlooking something. So I'm curious what others think of this too. Quote Link to comment Share on other sites More sharing options...
redbullmarky Posted December 9, 2007 Author Share Posted December 9, 2007 yup that's it. i mean i know how i'd implement it, but just wondering the longer term effects. i cant think of any right now, but having said that, i never thought there was anything wrong with my original spaghetti code from my earlier days until i needed to do some serious restructure work. I suppose there's the temptation to overly use it, which could probably introduce all the problems associated with globals, but i'm just looking at pretty much using it for "wrapper" objects for stuff that's accessible globally anyway (or should/could be). Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted December 10, 2007 Share Posted December 10, 2007 If I were to create such a device, I'd probably implement it like so: <?php class Loader { /** * Static var holds instantiated singletons for use in the site * 'className' => objectInstance */ private static $s_Singletons = null; /** * Magic method __call * Functionality depends on called name * getSingleton[Classname] - returns instantiated singleton * $a acts as constructor args * getInstance[Classname] - returns instantiated object * $a acts as constructor args * @param string called function name * @param array function arguments * @return mixed */ function __call($m, $a){ // Determine if we're instantiating an object $bSingleton = strpos($m, 'getSingleton'); $bInstance = strpos($m, 'getInstance'); if($bSingleton !== FALSE || $bInstance !== FALSE){ // Instantiating an object - get object name $name = $bSingleton !== FALSE ? str_replace('getSingleton', '', $m) : str_replace('getInstance', '', $m); // Actions differ depending on singleton or instance if($bSingleton !== FALSE && array_key_exists($name, self::$s_Singletons)){ return self::$s_Singletons[$name]; } $obj = new $name($a); if($bSingleton !== FALSE){ self::$s_Singletons[$name] = $obj; } return $obj; } } } ?> I left out error checking and other fancy features, like require_once()'ing the class file (if you're not using auto_load). Anyways, I'm sure that method adds some overhead, but it eliminates a lot of similar functions IMO and still gives you flexibility to use an existing object (like using an existing mysql connection) or requesting a new one. Opinions welcome. Quote Link to comment Share on other sites More sharing options...
redbullmarky Posted December 10, 2007 Author Share Posted December 10, 2007 ahh looks handy. thanks! you say: If I were to create such a device, I'd probably... as if you don't really do it this way at the mo, so how do you generally handle things like for your current stuff, with regards to passing around objects? Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted December 10, 2007 Share Posted December 10, 2007 I inherited the application I maintain at my full-time job and I don't currently do any freelance work, so I do it the way the original programmer implemented it. Lot's of: function blahblah(){ global $_db, $_sessObj; } I die a little inside every time I type it. I've been thinking about taking on some freelance work and almost thought about developing my own framework to do so, but then I realized I'd be so caught up in developing the framework I'd never do any freelance work. So I'm forcing myself to develop a site in cakePHP (despite it's faults) for the sake of overcoming my need to control every piece of software I use. I get some bright ideas here and there, but unfortunately I don't have the resources to implement them. I've got what I think is a pretty good one right now, but I think only the software giants have the resources to pull it off. I think it'd sweep the current business software model right off it's feet though. Quote Link to comment Share on other sites More sharing options...
448191 Posted December 10, 2007 Share Posted December 10, 2007 Other implementations have each class using their own factory - so instead of Factory::getSession(), it'd be more like Session::getInstance() - though wouldn't the latter mean that I've have lots of Singletons running all over the place rather than just the one? Yes, and there's a good reason for that too. It creates unnecessary dependencies on other classes. The classes Registry and Session are otherwise unrelated, so why create a dependency between them? So my question - are there any serious pitfalls (putting best-practice aside for a bit) of combining the registry with a factory, so i'd have factory methods such as getSession/getRequest (all under one factory roof), etc but where they actually store the instance for the next time they're required? Looking under the bonnet of Joomla, they do something sorta like this. Meh. Forget about terms like pitfalls or best practice. Basically, you need to know a choice's merits and disadvantages. I already mentioned the main disadvantage, but what's to gain? Not seeding your app with Singletons? With this proposed solution there would still be a method that can globally fetch instances from the class itself (either a singleton method or a constructor), otherwise the registry wouldn't be able either. What you really need to do is limit, if not eliminate, your dependency on the Registry. To quote a paper from ThoughtWorks I read recently: The benefit of OO is that it defines a unit of modularity which is internally coherent but has minimal coupling to the rest of the system. This makes it easy to modify software by changing how objects are composed together into an application. To achieve this flexibility in practice' date=' objects in a well-designed system should only send messages to their immediate neighbours, otherwise known as the Law of Demeter [14']. Note that the immediate neighbours of an object do not include objects whose references are returned from a call to another object. Programmers should avoid writing code that looks like: dog.getBody().getTail().wag(); colloquially known as a “Train Wreck”. This is bad because this one line depends on the interfaces and implied structure of three different objects. This style laces structural dependencies between unrelated objects throughout a code base. The solution is described by the heuristic "Tell, Don't Ask" [6], so we rewrite our example as: dog.expressHappiness(); and let the implementation of the dog decide what this means. A global Registry basically creates an object that's everybody's neighbour. Doing Registry::getSession(), or Registry::sessionFactory() is inadvisable, because the definition of how a Session object is created should be with/be assigned closed too the Session type itself. A Registry should be bothered only with storing and returning objects, not with their instantiation. Mind you, I have made this mistake myself in the past, because it seems convenient, but it really is not (at all). Lot's of: function blahblah(){ global $_db, $_sessObj; } I die a little inside every time I type it. LOL.. Try working with MediaWiki, then we'll talk. Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted December 10, 2007 Share Posted December 10, 2007 @448191, I'm curious what your thinking is on my approach. With this proposed solution there would still be a method that can globally fetch instances from the class itself (either a singleton method or a constructor), otherwise the registry wouldn't be able either. My method doesn't require the class be able to globally fetch instances of itself, although you could argue that they're not true singletons (but then others may argue that's a good thing). Registry::sessionFactory() is inadvisable, because the definition of how a Session object is created should be with/be assigned closed too the Session type itself. Again, my implementation doesn't require the factory / registry to know anything about how the object should be created. It merely passes on anything it received to the object's constructor. Quote Link to comment Share on other sites More sharing options...
redbullmarky Posted December 10, 2007 Author Share Posted December 10, 2007 Hmmm. John, when a class is written to pretty much act as a "wrapper" for data that's already available globally (sessions and request data being my first thoughts), is it still bad? If I had a single object do deal with, then passing it around doesn't seem an issue. But taking into account that most of my system will need access to the Request, Session, Config and DB objects, how is the best way to pass all of these around so they're readily accessible? Quote Link to comment Share on other sites More sharing options...
448191 Posted December 10, 2007 Share Posted December 10, 2007 Just to be postive, I'll start with the good news Again, my implementation doesn't require the factory / registry to know anything about how the object should be created. It merely passes on anything it received to the object's constructor. True, however you'd need to use to ReflectionObject::newInstanceArgs() to avoid putting restrictions on the contructors arguments. All nitpicking aside, this is a benefit of the solution you proposed. My method doesn't require the class be able to globally fetch instances of itself, although you could argue that they're not true singletons (but then others may argue that's a good thing). True, but (and this is the bad news), it also negates the the one benefit of singletons that has no ill effects: assuring a single instance. Wich (again) allows me put newest, ridiculously simple brainchild (the weirdest things come to life in that skull of mine) in the spotlight: the Singlestantiator. class Singlestantiator { private static $instantiated = false; public function __construct(){ if(self::$instantiated){ throw new SomeException('Too late buddy, somebody beat you to it.'); } self::$instantiated = true; } } Now you can just use a regular Registry, thus enforcing a central point of collection (just be very careful with your dependencies on it -- might not be worth it at all). Hmmm. John, when a class is written to pretty much act as a "wrapper" for data that's already available globally (sessions and request data being my first thoughts), is it still bad? I don't really like the word "bad", but if you insist: it's not the wrapper that is "bad", it's the data being globally available in the first place. The wrapper does not negate this problem either. If I had a single object do deal with, then passing it around doesn't seem an issue. But taking into account that most of my system will need access to the Request, Session, Config and DB objects, how is the best way to pass all of these around so they're readily accessible? A way that I've grown fond of is to make Gateways and Facades. Basically this enables to you to create a thin interface between different, loosely related layers of your application. For example I use what I call a Data Controller, which is a Gateway between the Data (integration) Layer and the Controller and (though I try to minimize this) View. Quote Link to comment Share on other sites More sharing options...
roopurt18 Posted December 10, 2007 Share Posted December 10, 2007 You're going to run into a problem with your current implementation of the Singlestantiator; I had this thought when I originally saw it but didn't bother to test my instinct until now. <?php define( 'NL', "\n" ); class Singlestantiator { private static $instantiated = false; public function __construct(){ echo 'Singlestantiator::__construct() entering...' . NL; if(self::$instantiated){ echo 'Too late buddy, somebody beat you to it.' . NL; } self::$instantiated = true; echo 'Singlestantiator::__construct() leaving...' . NL; } } class A extends Singlestantiator { public $member = 'A'; public function __construct(){ echo 'A::__construct() entering...' . NL; parent::__construct(); echo 'A::__construct() leaving...' . NL; } } class B extends Singlestantiator { public $member = 'B'; public function __construct(){ echo 'B::__construct() entering...' . NL; parent::__construct(); echo 'B::__construct() leaving...' . NL; } } header('Content-type: text/plain'); echo 'Testing...' . NL; $a = new A(); $b = new B(); ?> Outputs: Testing... A::__construct() entering... Singlestantiator::__construct() entering... Singlestantiator::__construct() leaving... A::__construct() leaving... B::__construct() entering... Singlestantiator::__construct() entering... Too late buddy, somebody beat you to it. Singlestantiator::__construct() leaving... B::__construct() leaving... I don't know. Maybe you didn't intend to use it as a base class for every singleton within your application, but then I wouldn't see much use for it. It's easily fixed if you change $instantiated to an associative array of parentClassName => bool. Quote Link to comment Share on other sites More sharing options...
448191 Posted December 11, 2007 Share Posted December 11, 2007 I don't know. Maybe you didn't intend to use it as a base class for every singleton within your application, but then I wouldn't see much use for it. It's easily fixed if you change $instantiated to an associative array of parentClassName => bool. It is intended to disallow repeated construction of a type, not an implementation. <?php class Cache { private static $instantiated = false; public function __construct(){ if(self::$instantiated){ throw new Exception('There can be only one cache.'); } self::$instantiated = true; } } class FileCache extends Cache { public $member = 'A'; } class MemCache extends Cache { public $member = 'B'; } //Test $a = new FileCache(); try { $b = new MemCache(); } catch(Exception $e){ echo $e->getMessage(); } ?> Quote Link to comment Share on other sites More sharing options...
Jenk Posted December 11, 2007 Share Posted December 11, 2007 I've not read all of the replies, but what you have described is the functionality of a Service Locator, Factory and Registry. All 3 are separate behaviours. However, it is common (though not everyone agrees good practice, including me) to combine them. I prefer to just use them as intended.. Service Locator example: <?php class ServiceLocator { public function locate ($name, array $args) { if (!class_exists($name)) { include('/some/path/to/class/files/' . $name . '.arbitray_extension'); } $reflect = new ReflectionClass($name); return $reflect->newInstanceArgs($args); } } ?> A registry example: <?php class Registry { private $_registry = array(); public function register ($name, $object) { $this->_registry[$name] = $object; } public function fetch ($name) { if (!isset($this->_registry[$name])) { throw new OutOfRangeException('There is no object registered under ' . $name); } return $this->_registry[$name]; } } ?> Factory: <?php class SomeClassFactory { private $_class = 'SomeClass'; private $_defaultArgs = array('some', 'args'); public function create (array $args = null) { if (is_null($args)) { $args = $this->_defaultArgs; } $reflect = new ReflectionClass($this->_class); return $reflect->newInstanceArgs($args); } } ?> If you want to combine the behaviours, then cascade like so: <?php $someObject = Locator::locate('SomeClassFactory')->create(array('new', 'args')); $registry->register($someObject); ?> Quote Link to comment Share on other sites More sharing options...
koen Posted December 11, 2007 Share Posted December 11, 2007 Yes, and there's a good reason for that too. It creates unnecessary dependencies on other classes. The classes Registry and Session are otherwise unrelated, so why create a dependency between them? One day you have 2 session handling classes, one with a DB, and with the regular session mechanisms, and you want to be able to choose between the dynamically. If you get your session class from the factory, there's only one place to get the session, which also decides which of the 2 session handlers will be used. What would be a good design now to include possible changes like this? Quote Link to comment Share on other sites More sharing options...
448191 Posted December 12, 2007 Share Posted December 12, 2007 Just use a simple Abstract Factory. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.