HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Handling views and templates

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
viewshandlingtemplatesand

Problem

My guts tells me that this isn't the proper way to handle views / templates, so that's why I'm asking you what could be done better.

Some background: This application I'm building now is for use in a school project with people that don't have much knowledge of PHP. Therefore I'm not using a framework like Laravel, or implementing a whole template engine, but I'm trying it to keep as simple as possible while still using classes and code separation.

The homeController:

Class homeController extends BaseController
{

    public function index()
    {
        $this->view->make("common/home");
    }
}


The View class:

Class View{

    public static $file;

    public function make($file){
        View::$file = $file;
        require_once DIR_VIEW . "default/template.php";
    }
}


The template.php file:


    Just an title

    

    
        
    

    


BaseController:

class BaseController
{

    public $load;
    public $url;
    public $view;

    public function __construct()
    {
        $this->load = new Loader();
        $this->url = new Url();
        $this->view = new View();
    }
}


The files are required by an loader class with this piece of code: (don't know if it ads some relevance)

foreach (glob(DIR_LIB . "*.php") as $filename) {
    require_once $filename;
}


The problem is probably that i'm using an static variable $file inside the View class. I don't think that that is right, but don't know any better way (besides using global(), but I know that that isn't the solution.

What do you think? Is this good enough or not even close?

Note: there will always be 1 file included, so that's why I thought that a simple require would succeed.

Solution

You're over-complicating it.

First the static variable isn't required, all the variables in the scope of include/require statements are available in the document being included or required. For example:

FileA.php

<?php
$variable1 = "Something";
require_once __DIR__ . "/FileB.php";


FileB.php

<?php
echo $variable1; // Outputs "Something"


So the $view variable will just be accessible in the template.php file. That said I think your code could be changed up just a bit and be a lot cleaner. I went through and tweaked it some with comments below. Feel free to ask any questions you may have after reading through it:

View.php

 in the header.php file.
        require __DIR__ . Views::DIR_VIEW . 'header.php';

        // Do this better, it's open to a directory traversal attack: http://en.wikipedia.org/wiki/Directory_traversal_attack
        require __DIR__ . Views::DIR_VIEW . $view . '.php';

        require __DIR__ . Views::DIR_VIEW . 'footer.php'; // Just require footer here

        return ob_get_clean();
    }
}


HomeController.php

view->make("common/home");
    }
}


common/home.php

Bam!  Wow! Such Templates!


header.php


    

    
        Home
    
    


footer.php


    
        © Copyright 2014
    

Code Snippets

<?php
$variable1 = "Something";
require_once __DIR__ . "/FileB.php";
<?php
echo $variable1; // Outputs "Something"
<?php
class View {
    const DIR_VIEW = "/views/";

    function make ($view, $data = array()) {
        extract($data, EXTR_SKIP); // Extracts key/value pairs from data as variables

        ob_start();

        // Just require header here, later on you could add another option to make for layout,
        // to use different headers footers if needed.  Just include everything up to
        // <div id="mainContent"> in the header.php file.
        require __DIR__ . Views::DIR_VIEW . 'header.php';

        // Do this better, it's open to a directory traversal attack: http://en.wikipedia.org/wiki/Directory_traversal_attack
        require __DIR__ . Views::DIR_VIEW . $view . '.php';

        require __DIR__ . Views::DIR_VIEW . 'footer.php'; // Just require footer here

        return ob_get_clean();
    }
}
<?php
class HomeController extends BaseController // Capitalize the H
{

    public function index()
    {
        // Add a return, have your router take the action method's return value and echo it.
        // This way your view class isn't just arbitrary printing things out, and will give you
        // flexibility later to have different types of returns (like for redirects you could
        // do Redirect::to('/'), similarly to laravel).

        return $this->view->make("common/home");
    }
}
<p>Bam! <?php echo $someVariable; ?> Wow! Such Templates!</p>

Context

StackExchange Code Review Q#69725, answer score: 3

Revisions (0)

No revisions yet.