Page 1 of 1

Better way to dynamically load instance w/ args

Posted: Fri Feb 11, 2011 6:25 pm
by Technocrat
I have a abstract class which has a getinstance function in. The problem I have is with arguments. Right now I am using an eval to pass those arguments to the class constructor. Is there a better way to do this.

See there HERE! part:

Code: Select all

abstract class Core {
	/**
	 * Arry of the class instances
	 *
	 * @var array
	 */
	static protected $_instances = array();

	/**
	 * Retrieves the singleton instance of this class.
	 * This is completely self loading
	 *
	 * @return instance of this class
	 */
	static public function getInstance() {
		//Get the called class so we know which class to load
		$class = get_called_class();

		//If there is not an already loaded instance
		if (!isset(self::$_instances[$class])) {
			//If there were no passed in arguments
			if (func_num_args() == 0) {
				//Make the new class instance
				self::$_instances[$class] = new $class();
			} else {
				//--------------- HERE! ------------------
				$args = func_get_args();
				$eval = 'self::$_instances[$class] = new $class(';
				for ($i = 0, $count = func_num_args(); $i < $count; ++$i) {
					$eval .= '$args['.$i.'],';
				}
				$eval = substr($eval, 0, -1);
				$eval .= ');';

				eval($eval);
			}
		}

		return self::$_instances[$class];
	}
}

Re: Better way to dynamically load instance w/ args

Posted: Mon Feb 14, 2011 6:56 pm
by Technocrat
/bump

Re: Better way to dynamically load instance w/ args

Posted: Tue Feb 15, 2011 11:52 am
by Technocrat
Well I found that you can use a reflectiveclass to accomplish this. The only issue then is I have to allow the constructor to be public which is fine just allows the ability to new a class instead of running the getInstance method.

Updated code:

Code: Select all

abstract class Core {
	/**
	 * Arry of the class instances
	 *
	 * @var array
	 */
	static protected $_instances = array();

	/**
	 * Retrieves the singleton instance of this class.
	 * This is completely self loading
	 *
	 * @return instance of this class
	 */
	static public function getInstance() {
		//Get the called class so we know which class to load
		$class = get_called_class();

		//If there is not an already loaded instance
		if (!isset(self::$_instances[$class])) {
			//If there were no passed in arguments
			if (func_num_args() == 0) {
				//Make the new class instance
				self::$_instances[$class] = new $class();
			} else {
				if (method_exists($class,  '__construct') === false) {
					trigger_error('Constructor for the class (' . $class . ') does not exist, thus you cannot pass arguments to it', E_USER_ERROR);
				}
				$args = func_get_args();
				$reflectionObj = new \ReflectionClass($class);
				self::$_instances[$class] = $reflectionObj->newInstanceArgs($args);
			}
		}

		return self::$_instances[$class];
	}
}

Re: Better way to dynamically load instance w/ args

Posted: Tue Feb 15, 2011 12:09 pm
by John Cartwright
eval() is the devil, and reflection is very expensive.

The has all the telltale signs of a design smell. Is your "core" class supposed to be inherited by any object that wishes to implement the singleton pattern? My first suggestion would be that the singleton getInstance() method should be implemented by the child class itself, which avoids the complexity of having to dynamically create instances with parameters, and allows for a proper inheritance hierarchy if necessary.

Re: Better way to dynamically load instance w/ args

Posted: Tue Feb 15, 2011 5:15 pm
by Weirdan
John Cartwright wrote:eval() is the devil, and reflection is very expensive.
Well, in this case it only called once per class - thus it's unlikely there would be a lot of calls to reflection here.

Re: Better way to dynamically load instance w/ args

Posted: Tue Feb 15, 2011 5:39 pm
by Technocrat
The instance is only called once. It appears at least in my testing that eval and reflection both "cost" about the same. Speed wise there is no difference.