I recently decided to see if I could make a functioning templating system. It works, but I feel like it is not all that great. What I'm looking for here is just general critique or ideas to improve. I'm new to this stackexchange, so if this is not the right place to do this, please let me know.
As requested below, I'll outline why I feel this templating system is not up to par. I drew inspiration from the Wordpress templating system. However, without having all the WP backend, its does not have nearly the same functionality (though it should be noted, I'm not trying to recreate WP, I just used it templating system as my inspiration). I supposed the most glaring piece missing is the lack of GET compatibility. You know how you can take www.site.com/?user=37 and make it www.site.com/user/37. Basically stripping out the GET requests and making it look nice. That's something that is missing. However, I feel like already my code in ungainly and a mess, and adding in exceptions for words or numbers that might be GET requests or might be filenames will only exacerbate the problem.
A problem I've run into is that, for some reason, sometimes it likes to grab my GET calls for the CSS and JS files and pipe it through the templating system, which obviously breaks the CSS and JS. Near as I can tell, it grabs the path/to/file.js and the templating system looks for templates/pages/path.to.file.js.php. I'm not 100% sure why it does this to the link and script includes of CSS and JS, but it is annoying. And one of the reasons that this system is sub-par.
To protect your sanity, I'll only include the important files. The Template class (shown) the request chained through public/index.php->public/bootstrap.php->utilities/main.php-> utilities/Template.php (thispage/requests->this/one)
/public
index.php
bootstrap.php
/templates
/layouts
header.php
home.php
404.php
page.php
multilevel.page.php
/utilities
main.php
Template.php
public/index.php
<?php require_once 'bootstrap.php'; ?>
<!-- if page is called with AJAX -->
<?php if(!empty($_SERVER['HTTP_X_REQUESTED_WITH']) && strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) == 'xmlhttprequest') : ?>
<?= Template::find(); ?>
<?php endif; ?>
<!-- otherwise, display -->
<!DOCTYPE html>
<html>
<head>
<?= Template::show('components.head'); ?>
</head>
<body class="<?= Template::classes(); ?>">
<header>
<?= Template::show('layouts.header'); ?>
</header>
<main>
<div class="container">
<?= Alert::display() ?>
</div>
<?= Template::find(); ?> <!-- main content -->
</main>
<footer>
<?= Template::show('layouts.footer'); ?>
</footer>
<?= Template::show('components.scripts'); ?>
</body>
</html>
utiltiies/Template.php
<?php
class Template
{
public static function show($file, $page = false)
{
if (!$page) {
$filepath = str_replace('.', '/', $file);
} else {
$filepath = 'pages/'.$file;
}
$template = ROOTPATH.'/templates/'.$filepath.'.php';
require_once $template;
}
public static function find()
{
$fileName = Template::getFileName();
if (Template::pageExists()) {
Template::show($fileName, true);
} else {
Template::show('404', true);
}
}
public static function getTitle()
{
if (Template::pageExists()) {
$fileName = Template::getFileName();
$file = ROOTPATH.'/templates/pages/'.$fileName.'.php';
$docComments = token_get_all(file_get_contents($file))[0][1];
if (strpos($docComments, "Title")) {
$woFront = str_replace('<!-- Title: ', '', $docComments);
$title = explode(' -->', $woFront)[0];
} else {
$title = ucwords(str_replace('.', ' ', $fileName));
}
} else {
$title = "Page not found";
}
return $title;
}
public static function classes($custom = false)
{
if ($custom) {
$custom = ' '.$custom;
}
$classes = Template::pageClass() . $custom;
return $classes;
}
private static function getFileName()
{
$URI = $_SERVER['REQUEST_URI'];
if ($URI === '/') {
$fileName = 'home';
} else {
$fileName = str_replace('/', '.', trim($URI, '/'));
}
return $fileName;
}
private static function pageClass()
{
$URI = $_SERVER['REQUEST_URI'];
if ($URI === '/') {
return 'home';
} elseif (Template::pageExists()) {
$className = str_replace('/', '-', trim($URI, '/'));
return $className;
} else {
return '404';
}
$fileName = Template::getFileName();
if (Template::pageExists()) {
$className = str_replace('/', '-', trim($fileName, '/'));
return $className;
} else {
return '404';
}
}
private static function pageExists($page = false)
{
if (!$page) {
$fileName = Template::getFileName();
} else {
$fileName = $page;
}
$file = ROOTPATH.'/templates/pages/'.$fileName.'.php';
if (file_exists($file)) {
return true;
} else {
return false;
}
}
}
1 Answer 1
The biggest problem is that you aren't building a templating system. You are building an entire PHP framework. Your code actually does very little templating. Instead it mainly does routing and asset loading. My general suggestion would be very simple: take some time to really understand MVC frameworks in PHP, because that is what you are really trying to build. The simplest way to do that is to learn one. I would start with something simple. Go download CodeIgniter, use, learn it, understand it and the many aspects of it, and then come back and try again.
I suggest this because your code shows that you have a general idea of what you are trying to do, but there are a very large number of important specifics that you are going to have to familiarize yourself with. What you have right now is a great way to get started with exploring these principles yourself, but it is a long way from being something that can be actually used in practice. So in short: keep working on it! Your next step is to work towards really understanding MVC frameworks in PHP. That is what you are actually trying to do (or at least, a good target to aim for), whether you realize it or not.
As for some specifics about your code:
1. It isn't really OOP
You are using classes, but you have static functions exclusively. This means that you could drop the class and just have functions and everything would work exactly the same. If you only ever use static functions you aren't really using OOP. Some better understanding of OOP principles and how the work/apply will be very helpful.
2. You need a real routing setup
Most common way to handle routing in PHP is with the front-controller pattern. You can find lots of articles about it. Wordpress uses it, as do just about all MVC frameworks in PHP. Yes, you will have to refactor everything.
3. File inclusion vulnerability
Your current code is vulnerable to a remote-file inclusion vulnerability. I can use this code to read any file on your computer that PHP has read access to. To get something like this production ready, you need to be an expert on defense-in-depth and web vulnerabilities in PHP.
4. No master classes!
You've basically got one class that just does everything. That runs very much contrary to the basic principles of OOP that make it so helpful in terms of long-term code management. Spend some time reading up on SOLID, its benefits, and how to implement it.
Those are just some highlights. Again, to emphasize, the best way to master concepts like this is to do exactly what you are doing: give it a try and look for feedback. So you're headed in the right direction! Next you want to learn about the guiding principles of good OOP (i.e. SOLID), principles/practical details of the MVC pattern, routing, and security.
Templatefunctions, yes? Or are you saying there is a way to merge that in? Not that I am opposed to the fundamental change, I just want to be sure I'm not going about it the hard way. \$\endgroup\$