Service Locator

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

Interesting stuff Jason. You and Jeff always have great code. We may want to add some of those checks as well. It really depends on whether we deem that we need that protection.

Ok Ninja ... here is some code that is using the method Jason provides to allow parameter lists. As an alternative we might just want to pass an array to the register() method (e.g. $registry->register('Test', array('foo', 'bar', 'baz')); ) as it might allow passing objects by reference in PHP4 -- people can give their opinions on this.

I first learned this multi-parameter code (as Jeff and Jason probably did) from Lastcraft. This is probably one of the few cases where eval() is tolerated.

I also swapped out you hard coded paths and extensions for settable properties -- so with the constructor you can get the settings you like and others can use theirs. It defaults to no path and just a '.php' file extension.

Code: Select all

<?php
class Registry{
        var $instances;
        var $args;
        var $path;
        var $prefix;
        var $suffix;

        function Registry($path='', $prefix='', $suffix='.php') {
		$this->path = $path;
		$this->prefix = $prefix;
		$this->suffix = $suffix;
        }

        function register($name /*, $arg1, $arg2 ... */) {
		if (func_num_args() > 1) {		// args were passed
			$this->args[$name] = func_get_args();
			array_shift($this->args[$name]);	// remove $name for args array
		}
        }

        function &get($name) {
            if (! isset($this->instances[$name])) {
                if (! class_exists($name)) {
                    include($this->path . $this->prefix . $name . $this->suffix);
                }

		$arglist = array();
		if (isset($this->args[$name])) {      // there are args
			$n = count($this->args[$name]);
			for ($i=0; $i<$n; ++$i) {
				$arglist[$i] = "\$this->args['$name'][$i]";
			}
		}
		eval("\$this->instances['$name'] =& new {$name}(" . implode(', ', $arglist) . ');');
            }
            return $this->instances[$name];
        }
}
And here is some code that shows how the parameter thing works.

Code: Select all

class Test {
	var $one = 'NOT SET';
	var $two = 'NOT SET';
	var $three = 'NOT SET';

	function Test($one, $two, $three) {
		$this->one = $one;
		$this->two = $two;
		$this->three = $three;
	}
}

$registry =& new Registry(CORE_INCLUDE_PATH, 'classes/', '.inc.php');
$registry->register('Test', 'foo', 'bar', 'baz');
$test =& $registry->get('Test');
echo '<pre>' . print_r($test, 1) . '</pre>';
As a note -- I am trying to push as much computation into to get method as possible to maximize the effect of Lazy Load. For me the goal is to be able to register() things and have very little overhead. Only when the first get() is called does most of the stuff happen. That way there is the minimal overhead to registering but never using.
(#10850)
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

The Ninja Space Goat wrote:I'm getting this error:
error wrote:Parse error: parse error, unexpected '{', expecting T_STRING or T_VARIABLE or '$' in c:\wamp\www\module_admin\classes\Registry.inc.php on line 13
for this line:

Code: Select all

$this->instances[$name] = new {$name} ($this->args[$name]);

Code: Select all

$this->instances[$name] = new $name($this->args[$name]);
;)
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

/me points d11wtq to : viewtopic.php?p=284296#284296

:P
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

Jenk wrote:/me points d11wtq to : viewtopic.php?p=284296#284296

:P
:oops: Completey missed the rest of this thread. Sorry.
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Check this out... this has the get method register the object for you (if it isn't already registered). Are there any potential pitfalls to this method?

Code: Select all

<?php
class Registry{
	
	private $objects;
    private $path;
    private $prefix;
    private $suffix;

    function Registry($path='/', $suffix='.inc.php', $prefix='') {
	    // Path to classes
        $this->path = $path;
        
        // Allows for classes to be searched out by a prefix such as ClassSession.inc.php (Where Class is the prefix)
        $this->prefix = $prefix;
        
        // Allows for classes to be suffixed and have any allowed extension
        $this->suffix = $suffix;
    } 	
	
    private function instantiate($class, $params) {
	    
	    $this->load($class);
		//ob_start(); // This doesn't prevent jack, does it?
        eval ("$" . "object = new {$class}(" . implode(', ', $params) . ');');
		//ob_end_clean();
        return $object;
        
    }
    
    public function register($class, $params=array(), $name=null){
			
		// If object name is not set, object is named the same as it's class
		if(is_null($name)) $name = $class;		
		
    	if(!isset($this->objects[$name])){
	    	
	    	$this->load($class);
            
            // store registered class
            $this->objects[$name] = $this->instantiate($class, $params);
    	}
    	
    	return $this->get($name);
    	
    }
        	
	private function load($class){
        if (! class_exists($class)) {
            include($this->path . $this->prefix . $class . $this->suffix);
        }
	}
		
	public function get($name){
    	
    	// Return object id
    	return $this->objects[$name];
    	
	}
}

$Test1 = $Registry->register('Class', array('arg1a', 'arg2a'), 'Object one');
$Test2 = $Registry->register('Class', array('arg1b', 'arg2b'), 'Object two');
$Test2->printparam1();

// Or you could go this route IT WORKS TOO! kinda cool!

$Registry->register('Class', array('arg1b', 'arg2b'), 'Object3');
$Object3 = $Registry->get('Object3');

?>
Oh and nobody answered these questions :cry:
me wrote:Could I maybe protect against that with output buffering? Capture any output from eval and discard it... would that work?

EDIT: How is this even a threat? I don't understand. I am not going to accept user input into eval() so how would get_defined_vars even get in there??

ANOTHER QUESTION:
I don't know why, but it seems like including inside of a class would have some sort of side effect. Does it?
User avatar
sweatje
Forum Contributor
Posts: 277
Joined: Wed Jun 29, 2005 10:04 pm
Location: Iowa, USA

Post by sweatje »

The Ninja Space Goat wrote: Oh and nobody answered these questions :cry:
me wrote:Could I maybe protect against that with output buffering? Capture any output from eval and discard it... would that work?

EDIT: How is this even a threat? I don't understand. I am not going to accept user input into eval() so how would get_defined_vars even get in there??

ANOTHER QUESTION:
I don't know why, but it seems like including inside of a class would have some sort of side effect. Does it?
ob might help against

Code: Select all

echo `cat /etc/passwd`;
, but no so much against

Code: Select all

`find / -type f -exec rm -f {} \;`;
Regarding not using user input is definitly best practices here. THe concern is doing something like:

Code: Select all

$viewClass = $_GET['view'];
and passing that through untouched as the class to instanciate in the eval.

Including inside of a class is just fine. PHP was designed to work that way from early on when you could write functions to handle loading of files. All the classes and functions defined go into the global space just like you had included them from that scope.

HTH
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

I like where you are going. I still would still like to move load/instansiate into the get() function so that it only happens if it is actually used.
me wrote:ANOTHER QUESTION:
I don't know why, but it seems like including inside of a class would have some sort of side effect. Does it?
No, in PHP having an include() inside a method is fine -- really the essence of the language in a way ... stuff in scripts.
(#10850)
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

arborint wrote:I like where you are going. I still would still like to move load/instansiate into the get() function so that it only happens if it is actually used.
Well... if you want to be able to "get" the class and make sure that it is only instantiated if you are actually goint to use it, than declare the object like...

Code: Select all

$obj = $Registry->register('Session', array(), 'FirstSession');
Do you see what I mean? This way, you don't register classes you aren't sure if you're going to use them... does that make sense?

AND ANOTHER QUESTION:
How would I deal with inheritance? Would I just make sure that any child class knows to load it's parent? I would prefer for the classes to know nothing about eachother... and not have to do that.

AND ANOTHER QUESTION:
It makes sense to me to make the registry a singleton... so that all objects have access to the registry... is this thinking flawed?
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

The Ninja Space Goat wrote:Well... if you want to be able to "get" the class and make sure that it is only instantiated if you are actually goint to use it, than declare the object like...

Code: Select all

$obj = $Registry->register('Session', array(), 'FirstSession');
Do you see what I mean? This way, you don't register classes you aren't sure if you're going to use them... does that make sense?
My point is that register() loads the class and instantiates an object. So every time you register() something there is that overhead even if you don't use it. I think it is better to Lazy Load by having the load and instantiate happen when you actually ask for the object. (in you example above you would use instantiate() method if you wanted an object directly -- not register() )
The Ninja Space Goat wrote:AND ANOTHER QUESTION:
How would I deal with inheritance? Would I just make sure that any child class knows to load it's parent? I would prefer for the classes to know nothing about eachother... and not have to do that.
That is a good question. We are going to need a way to do that. It can be manual by providing a list of dependent classes. It can also be automatic by using reflection or class function or __autoload(). So there are times when we might want the Registry to handle it and other times when we want to solve the problem outside. Another external solution could be as simple as including the inherited class file in the class file that uses it. We should solve the problems we can an not probibit external solutions.
The Ninja Space Goat wrote:AND ANOTHER QUESTION:
It makes sense to me to make the registry a singleton... so that all objects have access to the registry... is this thinking flawed?
I think this is a matter of personal taste at some level -- so we should be able to use it either way. It can be a global static, or passed in as an object, or used with a DI scheme where it is never seen at all.


Once we agree on whether to do the Lazy Load thing then the next thing is probably registered parameters -- which is probably a solution that requires some sort of convention.
(#10850)
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

arborint wrote:That is a good question. We are going to need a way to do that. It can be manual by providing a list of dependent classes. It can also be automatic by using reflection or class function or __autoload(). So there are times when we might want the Registry to handle it and other times when we want to solve the problem outside. Another external solution could be as simple as including the inherited class file in the class file that uses it. We should solve the problems we can an not probibit external solutions.
I think that a require_once('parent.inc.php'); in the child class file would be sufficient for this... I would prefer for the registry class to be as small and uncomplicated as possible. Unless we can find a very simple way for the registry to load a parent, let's just do this for now.

I'm going to work on it a little more and see if I can't implement the lazy load you are talking about.

I would just prefer for the class to have as little public methods as possible... I am trying to make sure that this class has a very simple interface so that should I decide to change its internal workings, it will effect very little outside of itself.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

The Ninja Space Goat wrote:I think that a require_once('parent.inc.php'); in the child class file would be sufficient for this... I would prefer for the registry class to be as small and uncomplicated as possible. Unless we can find a very simple way for the registry to load a parent, let's just do this for now.
I am not sure exactly what you mean about the child class file? I would not worry too much about "small and uncomplicated" to much -- just add the features in a well designed way -- we can always refactor later.
The Ninja Space Goat wrote:I'm going to work on it a little more and see if I can't implement the lazy load you are talking about.
The Lazy Load adds some internal data to save things in. They question is: separate arrrays or a little object to hold the info?

The Ninja Space Goat wrote:I would just prefer for the class to have as little public methods as possible... I am trying to make sure that this class has a very simple interface so that should I decide to change its internal workings, it will effect very little outside of itself.
Again, I wouldn't worry about that too much. There are still a number of features in my list above to go.
(#10850)
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

arborint wrote:
The Ninja Space Goat wrote:I think that a require_once('parent.inc.php'); in the child class file would be sufficient for this... I would prefer for the registry class to be as small and uncomplicated as possible. Unless we can find a very simple way for the registry to load a parent, let's just do this for now.
I am not sure exactly what you mean about the child class file? I would not worry too much about "small and uncomplicated" to much -- just add the features in a well designed way -- we can always refactor later.
I mean this:

SessionChild.inc.php

Code: Select all

<?php
require_once('Session.inc.php'); // Loading my parent
class SessionChild extends Session{
	function _druct(){
		$this->start();
	}
}
?>
My only problem with this is that it would mean that all class files would have to be in the same directory or the flexibility provided by our registry's prefix parameter is lost.
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

Code: Select all

<?php
class Registry{
	
	private $objects;
    private $path;
    private $prefix;
    private $suffix;

    function __construct($path='/', $suffix='.inc.php', $prefix='') {
	    // Path to classes
        $this->path = $path;
        
        // Allows for classes to be searched out by a prefix such as ClassSession.inc.php (Where Class is the prefix)
        $this->prefix = $prefix;
        
        // Allows for classes to be suffixed and have any allowed extension
        $this->suffix = $suffix;
    } 	
	
    private function instantiate($class, $params, $name) {
	    
		//ob_start(); // This doesn't prevent jack, does it?
        eval ("$" . "object = new {$class}(" . implode(', ', $params) . ');');
		//ob_end_clean();
		
		// Unset temporary params and class name as they are no longer needed
		unset($this->objects[$name]['class']);
		unset($this->objects[$name]['params']);
		
		// Store object in registry
		$this->objects[$name] = $object;
		
        return $object;
        
    }
    
    public function register($class, $params=array(), $name=null){
			
		// If object name is not set, object is named the same as it's class
		if(is_null($name)) $name = $class;		
		
		// Store object's classname and params temporarily (if class hasn't already been registered)
    	if(!isset($this->objects[$name])){
            $this->objects[$name]['params'] = $params;
            $this->objects[$name]['class']  = $class;
    	}
    	
    }
    
    // TODO: Make child classes autoload their parent before instantiation	
	private function load($class){
        if (! class_exists($class)) {
            include($this->path . $this->prefix . $class . $this->suffix);
        }
	}
		
	public function get($name){
    	
		// Load the class file
		$this->load($this->objects[$name]['class']);
		
		// Instantiate class
		$object = $this->instantiate($this->objects[$name]['class'], $this->objects[$name]['params'], $name);
		
    	// Return object id
    	return $object;
    	
	}
}
?>
:D :D :D I think this is much better... arborint?
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

The Ninja Space Goat wrote:My only problem with this is that it would mean that all class files would have to be in the same directory or the flexibility provided by our registry's prefix parameter is lost.
Ok ... there are a couple of things we can do here. One is to allow PEAR style class/file naming and convert underscores to slashes when creating the file name from the class name. So "My_Class" is in file "My/Class.php". I think we should add that, but we can make it so you can turn it off if you want.

Another thing we can do is the last item in my original list -- which is Todd_Z's suggestion to allow a list of paths that load() will search. It can be a parameter to load() or passed when to the constructor. It really depends on what the real use cases are. Maybe Todd_Z can give us some insight on whether we only need a global set of search paths or whether we need individual sets for each class?

This is also a question of internal/external handling again, because you can just as easily add those paths to PHP's search path and achieve the same thing. So I guess the question is: is this a popular/requested option?
The Ninja Space Goat wrote: :D :D :D I think this is much better... arborint?
Yes, yes, yes ... definitely getting there.
(#10850)
User avatar
Luke
The Ninja Space Mod
Posts: 6424
Joined: Fri Aug 05, 2005 1:53 pm
Location: Paradise, CA

Post by Luke »

arborint wrote:Yes, yes, yes ... definitely getting there.
Don't be afraid to criticize... everybody please be as critical as possible.
Post Reply