views:

87

answers:

4

I understand that global variables should be avoided generally. As a beginner, I'm trying to understand whether making a $view variable global in a dynamic web program built following MVC principles is one of those cases where globals are a good idea.

In my program, I create the $view (as an object that contains an empty array) in index.php, and turn it into a global in the controllers that use it.

Thanks!

JDelage

+1  A: 

Making a variable global is not a good idea when you using an MVC patterns in a project. There are other solutions that will make use of your MVC more wisely.

If you should have a single use of a resource, use a singleton pattern. Something like

class My_View {

   private $_instance;

   static public function getInstance() {
     if (null === self::$_instance) {
        self::$_instance = new self();
     }
     return self::$_instance;
   }

   private function __construct() { }

   public function __clone() {
      trigger_error("Cannot make copy of this object", E_USER_ERROR);
   }

   // add public methods and/or properties here...
}

and get that view object anywhere with

$view = My_View::getInstance();

This way, you don't have global variables and you are using best OO practices.

However, as others have pointed out, having a singleton view is not necessarily a good idea... A probably better approach would be to make your dispatcher code create a new view and set it to the controller being called, so the controller would have a view available and access it directly.

The view (not being a singleton) could also be a public property, or accessible though a public method of an application singleton; My_Application::getInstance()->getView(); that could also hold the current configuration, paths,

The name My_view suggested is merely an example. Using some sort of naming convention helps organize the code and helps getting rid of all those include/require calls in your script headers. This is not in your question's scope, however for clarity's sake, I'll briefly explain :

In your bootstrap, you declare your autoloading function (as defined in the PHP manual) :

// My_Application.php located in /path/to/lib/My/Application.php
class My_View {

   private $_instance;

   static public function getInstance() {
     if (null === self::$_instance) {
        self::$_instance = new self();
     }
     return self::$_instance;
   }

   private $_basePath;
   public $view;

   private function __construct() { }

   public function __clone() {
      trigger_error("Cannot make copy of this object", E_USER_ERROR);
   }

   public function initAutoloader($base) {
      $this->_basePath = $base;
      spl_autoload_register(array($this, 'autoload'));
   }

   public function autoload($name) {
      require $this->_basePath . str_replace('_', '/', $name) . '.php';
   }

   // get the application global view
   public function getView() {
      if (null === $this->view) {
         $this->view = new My_View();
      }
      return $this->view;
   }
}
// perhaps have the parameter being a constant. I.g. APP_BASE_PATH
My_Application::getInstance()->initAutoload('/path/to/lib/');

And simply include the file '/path/to/lib/My/Application.php' and when you will access My_View, the load function will be called with $name = 'My_View' and the function will simply require the file '/path/to/lib/My/View.php' for you. This is not much for one file, but if all classes are namespaces like so, you only need one include (Autoloader) and everything else is loaded automatically.

Yanick Rochon
BUT: using singletons is usually bad practise, safe for a few special cases.
Jacco
Singletons are just an elaborate semantic workaround to have global variables by another name.
mario
it is bad practice only when this pattern becomes ones pattern of choice :) Otherwise (like in this case) it's a far better solution than using a global variable. Instead of having two or more singletons, one should create a unique singleton holding whatever needs to be shared. ...Many scenarios could be set in example, but the principle remains; it should not be the first pattern that comes to mind, I agree.
Yanick Rochon
@mario, the difference is that you cannot 'set' a singleton, whereas you can easily substitute a global variable. Singletons ensure some form of integrity in the application.
Yanick Rochon
It's not the Singleton that ensures the integrity of a class. It's the class itself. Singleton's are not a *best OO practise*. They are a code smell. `My_View` is a hard coded classname. Thus it is a dependency on the global scope (or current namespace). It's no way better than using a global variable. In fact, it is worse, because a `global $view` can much more easily be substituted with a Mock than a hardcoded class. That's not to say use globals. Both suck. Also, why should there be only one View instance?
Gordon
@Gordon, I said it ensures some form of integrity in the application, I never talked about class integrity. And how a class can be hard coded in the global scope? ..it's a class name! PHP keeps a registry of class names and where the code is located, I could argue that a function in the global scope... is also in the global scope, even worst! There can be no real structure with global functions; no encapsulation, no autoloading, but a bunch of include and require statements throughout the application. Wordpress and Drupal are a nightmare in that sense; "Where that xyz function's body is..?"
Yanick Rochon
@Gordon, the question makes reference of using an MVC pattern, suggesting another pattern is simply logical. And since "making a $view variable global in a dynamic web program" was part of the question description, the Singleton pattern is suggested. Also, this answer organizes the different component in namespaces, thus allowing autoloading of the class. Finally, I agree that a view should not have a single instance. I work with ZF and despite that I usually work with a single registered view instance, the class Zend_View itself is not a singleton, but this was not intended by the OP.
Yanick Rochon
How is that logical? MVC has nothing to do with the Singleton pattern. It's not even a web presentational pattern. A Singleton only ensures that a specific class can only be instantiated once. That's all. This wasn't asked for by the OP. The Singleton being available everywhere is solely due to it being accessed *statically* through the `getInstance()` method. But - if you are not using namespaces - doing `MyClass::foo()` is no different from using a regular function, e.g. `myclass_foo()`, because a static invocation is a call to the global namespace then, with the same risk of clashing.
Gordon
@Yanick Rochon, you state: "this answer organizes the different component in namespaces". But in fact, you organize the different components in classes. a class != a namespace. Although both offer a form of encapsulation.
Jacco
@Jacob, since PHP 5.3 is not implemented in every web hosting just yet, this is a substitute to "My\View". It's a naming convention that has been adopted by Zend and it works very well. But in any case, who am I to argue if someone wants a bunch of global functions?
Yanick Rochon
@Yanick: Shouldn't `static public getInstance()` really be `static public function getInstance()`?
JDelage
@JDetage, indeed! thank you for spotting that out.
Yanick Rochon
+1  A: 

Global variables should be avoided, true. They are however useful for holding global state - that doesn' lead to intercoupling - as in specifically a $config array/object.

But in your case, I'm not sure the $view is a good idea. Firstly, I believe what you are assembling is output data. The "view" in MVC is more of a output template, what I believe your index.php is? If so, then you are actually assembling the $model. Uncertain.
It's theoretically cleaner to pass this variable to the controllers explicitely then. If it's however really only that one global variable, and you promise not to overdo it, passing it around as global is ok.

But you could also follow the mentioned singleton pattern. However I'd recommend the procedural variant, because it's cleaner:

function view() {
    static $view;
    if (!isset($view)) { $view = new ArrayObject(); }
    return $view;
}

This way you could use $view=view(); to localize it in your controllers, keep using it as array $view["xy"]= and as object $view->xy= at the same time.
Or just write print view()->title; in the template.

mario
+3  A: 

In my program, I create the $view as an empty array() in index.php, and turn it into a global in the controllers that use it.

Why? If you controller needs the $view then just pass it in via the constructor or a setter. Dependencies on Globals can always be resolved by using Dependency Injection.

// index.php
$view = array();
$controller = new Controller($view);
$controller->doAction();

Also, please reconsider if the View should be just an array. The View has to be rendered at some point. Rendering is a responsibility I'd see on the View. An array cannot render itself, so I assume you are doing it elsewhere. That would be a violation of the Single Responsibility Principle. I'd rather do something along the lines of (simplified):

// index.php
$view = new View;
$controller = new Controller($view);
$controller->doAction();

// controller.php
...
public function doAction()
{
    ...
    $this->view->setTemplate('/path/to/template');
    $this->view->setVar('foo', 'bar');
    $this->view->render();
}
...
Gordon
+1  A: 

A solution to this problem I've seen is having controllers extend from a base controller. Within that base controller a $data property is instantiated; new view data is then appending to this property. For example:

class PageController extends BaseController {

    function IndexAction() {
        $this->data['title'] = "Page Title";
        $this->data['content'] = "<p>Page content.</p>";

        $this->render();
    }
}

The render method (which could be loaded automatically after a method has ran, rather than being explicitly) would then assemble a template using the data held within the $data array.

Hope this helps and you're able to implement a working solution.

Martin Bean
Yes, this is what I have done.
JDelage
Well then that should be fine, but is also not using "global" variables. Not sure if that's you not understanding the above example, or the definition of global in PHP.
Martin Bean