Bad code, bad data flow, good idea?

May 14th, 2007 • Related • Filed Under

If you're new here, you may want to subscribe to my RSS feed. Thanks for visiting!

When working with any kind of MVC framework you find yourself at some points thinking, I could really have designed this better. This may be that your views are cascading badly, your model has code in it that should be in the controller or maybe it is just plain bad.

One of these situations occurs when you suddenly that find you dont have access to data you want, or you can’t pass back (or forward) data as part of the common flow of your application. I have been reading a lot recently about various design patterns within MVC’s (helpers, cascading, composite, etc..) all which I use in some form depending on requirements.

I wanted an easy way to pass data forward<>backward when completely out of context (and it will happen!) which did not rely upon global variables. The solution was to create a singleton class which uses references to bind a name to a already defined variable.

So as an example. In my overall site.tpl.php view, I have the variable $javascript. Which contains an array of javascript files I wish to include. 

$view->stylesheets = array();
varmap::attach($view->javascript,"javascript");

It is important that the variable is defined first as you cannot get a reference to an unset variable. Now we have attached we can then use another function anywhere in your code to add any extra required javascript files without trying to pass back anything (or use a global). 

varmap::add("javascript","other_required.js");

This has now actually altered the data back in the $view->javascript variable because the singleton class is using a reference. I have found this extremely useful within form construction classes where I am 3 or 4 levels deep in classes and the class has a requirement to include some extra resource within my header.

Now I could just use a $GLOBAL, but this requires the ‘view.tpl.php’ to read from a global (yuck) or for the code to extract the data back from global into the $view->javascript after we had finished building our forms (again, not very desirable). Either way my solution is hopefully more elegant and less prone to making mistakes as the code for attaching is located in the same place as the variable creation.

Just to give the final part of the example here is the section from my ‘view.tpl.php’ that actually outputs the html.

if(isset($this->javascript)) { 
	foreach($this->javascript as $sheet) {
		echo '<script type="text/javascript" src="/public/scripts/'.$sheet.'">
			</script>';
	}
}

Below is the actual singleton class (stripped of comments), it is very simple and could (should) be extend to be made more useful to your own requirements. I have included the one extra ‘helper’ method which I have demo’ed above which is ‘add’ this just checks if we are adding to an array or to a string and performs the correct operation. 

class varmap
{
  	private $vars = array();
  	static $instance = null;
 
  	public function getInstance()
  	{
  		if(self::$instance == null) {
  			self::$instance = new varmap;
		}
		return self::$instance;	
	}
 
  	public static function attach(&$var,$bind)
  	{
  		$gl = self::getInstance();
  		$gl->vars[$bind] = &$var;
	}	
 
	public static function set($bind,$value)
	{
		$gl = self::getInstance();
		$gl->vars[$bind] = $value;	
	}
 
	public static function get($bind)
	{
		$gl = self::getInstance();
		return $gl->vars[$bind];
	}	
 
	public static function add($bind,$value)
	{
		$gl = self::getInstance();
 
		if(is_array($gl->vars[$bind])) {
			if(!in_array($value,$gl->vars[$bind])) {
			    $gl->vars[$bind][] = $value;
			}
		} else {
			$gl->vars[$bind] .= $value;
		}
	}	
}

Hopefully someone will find this useful, if you do please write back and tell me how you have used it or improved it.

There Are 10 Responses So Far. »

  1. Trackback by PHPDeveloper.org on 15 May 2007:

    Nick Halstead’s Blog: Bad code, bad data flow, good idea?…

    …

  2. Comment by stefan on 15 May 2007:

    It could be me, but since you never call the getInstance(), the varmap class itself is not a Singleton but a Registry class ;)

    Or am I now misreading your code?

  3. Comment by admin on 15 May 2007:

    Each of the static functions references the getInstance() and yes within this example I never return that instance outside of the class itself. So depending on how you decide to use it, its either a singleton or a registry class :) but I only put this forward as an example, as I am sure everyone would want to take this further depending on their needs. This may mean grabbing the instance, then calling non-static methods.

  4. Pingback by developercast.com » Nick Halstead’s Blog: Bad code, bad data flow, good idea? on 15 May 2007:

    [...] he realized that the part of the application he was working on could have been coded better. In this instance, he found a spot where he needed to pass data backwards and forwards through the app without [...]

  5. Trackback by Webmaster resources and websites on 17 May 2007:

    Bad code, bad data flow, good idea?…

    When working with any kind of MVC framework you find yourself at some points thinking, I could really have designed this better….

  6. Comment by bosmeeuw on 21 May 2007:

    You’re declaring “static $vars” but further down accessing this variable as an instance variable: $gl->var. This should throw a notice. I think you can forego the instance entirely and just use self::$vars.
    You have then created a nice replica of $GLOBALS: a globally accessible key/value hash. Maybe you should instead adopt a full OOP style of writing PHP and properly encapsulate your “global variables” in Singletons with meaningful class names and members.

  7. Comment by Nick Halstead on 21 May 2007:

    Bosmeeuw, your completely correct! A slight oversight, that should have been a private. Now fixed on the post. Thanks for pointing it out.

    I do already use mostly singletons for all my models, but this solution was to solve a problem of variable passback within a MVC framework. And remember this is not a variable storage like $GLOBALS, it is using references to allow you to set variables outside of your current scope which is totally different.

  8. Comment by Steve Clay on 25 May 2007:

    So varmap is essentially a global reference store. Here’s another implementation:

    class RefStore { public static $refs = array(); }
    // usage:
    $foo = array(1, 2);
    RefStore::$refs['foo'] =& $foo; // set
    $foo2 =& RefStore::$refs['foo']; // get

    Reason I’d prefer this over varmap: 1. no methods to call (faster) 2. crystal clear what is happening (you’re setting a reference, getting a reference) 3. You can do whatever you want with your retrieved local reference, rather than having to add varmap methods to add/subtract/wizz/bang/etc. 4. Writing one with your desired namespace prefix is cake.

    Reason why this still beats something like: $GLOBALS['RefStore']: If the code you’re integrating with is using a RefStore, the fatal error tells you quickly.

  9. Comment by admin on 26 May 2007:

    Thanks for that Steve, I think its a very nice compact way of implementing a reference store. The only downside to the compactness is you have to remember to use & every time you use it. By encapsulating it within the functions you are removing programmer error, plus you do get the advantage of being able to extend the functionality.

  10. Trackback by Anonymous on 4 April 2008:

    world free shemale sex top vadios clips…

    world free shemale sex top vadios clips…

Post a Response