tags:

views:

101

answers:

3

I've been working on my own little framework for a while for my own benefit, constantly going back over the code as I learn new stuff. As you might expect, I have a Registry object that's used by pretty much every other object.

At present, the most basic object (AFObject) is set up a little like this

absract class AFObject {

    var $_registry;

    function __construct(){
        $this->_registry = AFRegistry::getInstance();
    }

}

So every object will now contain a local reference to the Registry. So if I have hundreds of objects instantiated at 1 time, that's hundreds of references to the singleton. But would it be more or less efficient to always refer to the Registry directly like this...

class AFRouter extends AFObject {

    function someMethod( $bar ){
        AFRegistry::$foo = $bar;
    }

}
A: 

I don't think you should think about efficiency in this case (since 100 references really isn't a problem, and is a bit premature optimization). But consider what is most elegant in your code. Also, consider if you need a singleton (could it be implemented as a static class?). I would maybe choose to use your second case, since that makes your code a bit more obvious (at least I think so).

In that case it would be

class AFRouter extends AFObject {

    function someMethod( $bar ){
        AFRegistry::getInstance()->$foo = $bar;
    }

}

Or if you encapsulate your property:

class AFRouter extends AFObject {

    function someMethod( $bar ){
        AFRegistry::getInstance()->setFoo($bar);
    }

}
Yngve Sneen Lindal
+2  A: 

In my opinion, "registry" type of classes kind of smells.

Since you mentioned you were doing this for learning and getting better, have you ever considered completely eradicating your registry class and taking another approach? Perhaps pushing required data to class constructors instead of pulling it from the inside of the class?

I'd leave out option 1 (abstract base class), because then all your classes becomes dependent on some other classes...

Using a static class like Yngve Sneen mentioned would be the best approach in my opinion if you do wish to keep a registry setup.

Something like: registry::set('var1', $var1); $var1 = registry::get('var1');

Mario
+1  A: 

Consider this:

class AFRouter extends AFObject {
  function someMethod($bar) {
    global $af_registry;
    $af_registry->setFoo($bar);
  }
}

or even:

class AFRouter extends AFObject {
  function someMethod($bar) {
    af_registry_set('foo', $bar);
  }
}

Bar the syntax, there is essentially no difference between this and your current solution.

Yes, that means that your registry is essentially a global variable. And yes, there are problems with global variables. A better option would be to pass in the dependencies.

troelskn