3

I'm rewriting a lot of my old mucky spaghetti code and am currently trying to get to grips with classes, functions and loosely rebuilding my site around MVC model.

However, I cannot get my header template include, to reference user account details which are autoloaded by my main config file and I think I'm missing out a very important step.

The error message is Fatal error: call to member function is_loggedin() on a non-object..., so I'm guessing that the include performed in template.class.php cannot access the account.class.php functions.

UPDATE: peforming a var_dump(get_included_files()) within header.tpl.php shows that both account.class.php and template.class.php are included (in that order). I also tried manually including account.class.php at the top of header.tpl.php to see if it would make any difference...it didnt. HELP :(

I should also note that I can call $_account->is_loggedin() from index.php with no issues. Jst not from within the included file header.inc.php.

Chances are I'm going abut this all wrong, so below is a simplified write-up of my code if anyone can offer some pointers:

index.php

<php require 'defaults.php'; ?>
<html>
<head>
...
</head>
<body>
<?php $_template->load('header'); ?>
....
</body>

defaults.php

session_start();

// define path settings
// connect to database, memcache
// lots of other stuff....

//autoloader for functions
spl_autoload_register(function ($class) {
  if (file_exists(CLASS_PATH.DS.$class.'.class.php'))
  {
    include CLASS_PATH.DS.$class.'.class.php';
  }
});

$_account = new account();  // autoload user account stuff
$_template = new template(); // autoload templates

account.class.php

class account
{
  private $db;

  public function __construct($db) {
    $this->db = $db;
  }


  public function is_loggedin() {
    // do various session checks
  }
}

template.class.php

class template
{
  public function load($template)
  {
    if (file_exists(TPL_PATH.DS.$template.'.tpl.php'))
    {
      include TPL_PATH.DS.$template.'.tpl.php';
    }
  }
}

header.tpl.php

<div id="header">
<?php if($_account->is_loggedin() == true): ?>
  <p>logged in</p>
<?php else: ?>
  <p>not logged in</p>
<?php endif; ?>
James
  • 656
  • 2
  • 10
  • 24
  • Just before using the class, do a `var_dump(get_included_files());` to see if your class files are included. The error you have described usually comes from a non-existing class. Either your autoloader register is failing or there is a bug somewhere in the inclusion of the class files. Debug it step by step and you should be a bit closer to resolving the issue. – Dzhuneyt Sep 09 '13 at 11:03
  • Your application model can be improved and your code style is a bit oldschool PHP4 like, but you do a good start for a newcomer to classes, patterns and models. For a hint, I would extend the `if` clause in template.class.php by something like `else { throw new Exception("include file not found"); }` - to see if the file actually gets included. – Daniel W. Sep 09 '13 at 11:04
  • Yeah I'm very oldschool unfortunately, but trying to break the habbit. I have added the exception to both the autoloaer in defaults.php and also to the template.class.php, but I'm not getting any errors. The template is definitely included as when i visit the index, i see the contents of the header template....but beyond that I'm not sure – James Sep 09 '13 at 11:19
  • I would start by `var_dump`ing `$_account` right before the line that throws the error, so you can see exactly what it thinks the variable is. – brianmearns Sep 09 '13 at 12:16

1 Answers1

2

The $_account that you are trying to access is a variable in the function scope of your template::load method, the $_account that you initialized is a global variable.

If you want to use a global variable you should either declare it with global $_account within the function or use $GLOBALS['_account'].

A simple template example (the rest is copied from your source):

Template.class.php:

interface IApiHandler {
    public function printContent();
}

SomeTemplate.class.php:

class SomeTemplate implements Template {
    public function printContent() {
        $account = Account::getCurrent();
        ?>
        <div id="header">
        <?php if($account->is_loggedin() == true): ?>
            <p>logged in</p>
        <?php else: ?>
            <p>not logged in</p>
        <?php endif;
    }
}

Account.class.php:

class Account {
    private static $current = null;
    public static function getCurrent() {
        if(self::$current === null) {
            self::$current = new Account();
        }
        return self::$current;
    }
    public function __construct() {
        //account init...
    }
    public function isLoggedIn() {
        return rand()%2;
    }
}

defaults.php:

session_start();

// define path settings
// connect to database, memcache
// lots of other stuff....

//autoloader for functions
spl_autoload_register(function ($class) {
  if (file_exists(CLASS_PATH.DS.$class.'.class.php'))
  {
    include CLASS_PATH.DS.$class.'.class.php';
  }
});

$headerTemplate = new HeaderTemplate();//not included in the example...
$bodyTemplate = new SomeTemplate();

index.php

<php require 'defaults.php'; ?>
<html>
<head>
...
</head>
<body>
<?php $headerTemplate->printContent(); ?>
<?php $bodyTemplate->printContent(); ?>
</body>

It should also be noted that this is missing the C from MVC. It is only an example of how to make templates as classes.

Usually the index (or in this case default.php) only needs to decide which controller is suppose to handle the request. The controller then has to decide (or default to) which template should be used (if any).

Also it is better if all the html is contained in the templates, no output should be printed before the controller handles the request.

Vatev
  • 7,493
  • 1
  • 32
  • 39
  • is using a global a 'hack' to get around a problem i've introduced in my model? If so, is there a betetr way to get around the problem? – James Sep 09 '13 at 12:21
  • It is as much a hack as global variables in general. It is better to use static class vars instead of globals, they are still global but at least are organized by classes and IDE's handle them better. If you want to completely avoid globals you will have to pass it as a parameter all around the place (which option is better is a whole topic by itself with its own religious debates). – Vatev Sep 09 '13 at 12:28
  • Just to expand on this a little: when you use `include` or `require`, like you did in the `template::load`, it includes the file directly in place, which is this case means it's part of the `load` function's body. That's probably not what you really want, and it will likely bite you in other (similar) ways as well. For instance, if you define any functions of classes inside the included file, they will not have scope outside the function either. I'm not really sure what the solution is for this though, i.e., how to include variable files. Seems like it should be common, though. – brianmearns Sep 09 '13 at 12:36
  • A simple solution to the confusion would be to define your templates as classes (with common a base class or an interface for them) and include them via the autoloader. That way there will be less scope confusion and your IDE will also be able to correctly determine the variable scope. – Vatev Sep 09 '13 at 12:44
  • Thanks for all the help Vatev. I thing i get the gist of what you are saying, but I can't quite get my head around it. If you get a chance I'd really appreciate an example. – James Sep 09 '13 at 13:15