4
\$\begingroup\$

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:

<!DOCTYPE html>
<html>
<head>
    <title>Just an title</title>
</head>
<body>
    <?php require_once 'header.php'; ?>

    <div id="mainContent">
        <?php require_once DIR_VIEW . View::$file . ".php"; ?>
    </div>

    <?php require_once 'footer.php'; ?>
</body>
</html>

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.

\$\endgroup\$
  • \$\begingroup\$ And how are you building and accessing the classes? \$\endgroup\$ – Ismael Miguel Nov 13 '14 at 16:07
  • \$\begingroup\$ Just a tiny nitpick - Class should be lowercase, as in: class homeController extends BaseController \$\endgroup\$ – jsanc623 Nov 13 '14 at 16:13
  • \$\begingroup\$ @IsmaelMiguel, added the baseController, do you need something else? \$\endgroup\$ – Mathlight Nov 13 '14 at 16:23
  • 1
    \$\begingroup\$ @jsanc623 I do know that PHP doesn´t care at all. But it's best to follow any guide lines that the community are also using. \$\endgroup\$ – Mathlight Nov 13 '14 at 17:02
  • 1
    \$\begingroup\$ @Mathlight I used that trick to create a session manager. It really is useful for some other projects, like yours. You should explore it a little bit more. I'm glad you are learning a lot: that means we are helping you to improve. \$\endgroup\$ – Ismael Miguel Nov 13 '14 at 17:32
3
\$\begingroup\$

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

<?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();
    }
}

HomeController.php

<?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");
    }
}

common/home.php

<p>Bam! <?php echo $someVariable; ?> Wow! Such Templates!</p>

header.php

<!DOCTYPE html>
<html>
<head>
    <title><?php echo isset($title) ? $title : 'Your site'; ?></title>
</head>
<body>
    <nav class="site-nav">
        <a href="#">Home</a>
    </nav>
    <div id="mainContent">

footer.php

    </div>
    <footer class="site-footer">
        &copy; Copyright 2014
    </footer>
</body>
</html>
\$\endgroup\$
  • \$\begingroup\$ This sounds very promising. I'll try to implement it and fiddle around with it. Also i'll leave this question open for just some more time so that others can react to. Maybe they come with some interesting stuff to ( so that i can learn ;P ) \$\endgroup\$ – Mathlight Nov 13 '14 at 16:31
  • \$\begingroup\$ I just have one thing to point out: You are using mixed quotes. Stick into single (') or double (") quotes though your code. I always choose single quotes for performance reasons. But that is a simple micro-optimization and you can safely ignore it. But for readability you should stick with the quotes you prefer. \$\endgroup\$ – Ismael Miguel Nov 13 '14 at 16:37
  • \$\begingroup\$ Also, my first question regarding your code: why do you use an const in the View? I mean this part: const DIR_VIEW = "/views/"; Why don't U use an define or simple $ variable? \$\endgroup\$ – Mathlight Nov 13 '14 at 16:50
  • \$\begingroup\$ @Mathlight a define creates a global constant. He is keeping the data self-contained. The DIR_VIEW will always be the same and won't change in your code. You could use a global constant, but that doesn't give any guarantee that it will actually exist. It also, as a side effect, will increase the consistency of your code. This is my interpretation and I might be wrong about this. \$\endgroup\$ – Ismael Miguel Nov 13 '14 at 16:54
  • \$\begingroup\$ @IsmaelMiguel Well, I define it in an config.php which is loaded at the very beginning. But besides that, why would that be more consistent then? ( sorry for all these question, but I'm trying to learn the maximum now if possible :D \$\endgroup\$ – Mathlight Nov 13 '14 at 17:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.