[web] [ PHP ] Is this code good?

I asked in a previous thread about creating a page-loading mechanism for a website. Well, I put some more thought into it and I got a working solution. I just need to know if this is good code or not, and how it can be improved.

[source lang=cpp]
// index.php
<?php

require (dirname(__FILE__) . "/brick/include.php"); 

Session::Start(); 
Error::SetHandler("DEFAULT", E_ALL ^ E_NOTICE);
Error::ReportDefault();

$page = new Page($_GET['page']);

?>

[source lang=cpp]
// /brick/include.php
<?php

/* Base Directories */
define ("WWW_ROOT", $_SERVER['DOCUMENT_ROOT']); 
define ("BASE", dirname(__FILE__)); 


/* Directories With Other Include Files */
define ("CORE", BASE . "/core/"); 
define ("MODULE", BASE . "/module/"); 
define ("PAGE", BASE . "/page/"); 
define ("SNIPPET", BASE . "/snippet/"); 


/* Include Core Files */
require (CORE . "utility.php"); 
require (CORE . "error.php"); 
require (CORE . "session.php"); 
require (CORE . "database.php"); 
require (CORE . "module.php"); 
require (CORE . "page.php"); 


/* Include Modules */
require (MODULE . "user.php"); 
require (MODULE . "inbox.php"); 
require (MODULE . "blog.php"); 
?>


// /brick/core/page.php
<?php

class Page
{
	const COMMAND_SYMBOL = "@"; 
	const VAR_DELIM = ":"; 
	const DEFAULT_PAGE = "home"; 
	const ERROR_PAGE = "404"; 
	const DEFAULT_TITLE = "No Title"; 

	private $title = Page::DEFAULT_TITLE;


	function __construct($page = "")
	{
		$this->load((!empty($page)) ? $page : Page::DEFAULT_PAGE); 
	}
	
	function parse($line)
	{
		$line = trim(str_replace(array(Page::COMMAND_SYMBOL, "{", "}"), "", $line)); 	
		$exp = explode(":", $line, 2); 
		
		switch (strtolower(trim($exp[0])))
		{
			case "snippet":
				{
                                    return $this->output($this->get_contents(SNIPPET . trim($exp[1]) . ".php"));
                                    break;
				}

			case "output":
				{
                                    return trim($exp[1]);
                                    break;
				}
			
                        case "title":
                                {
                                    $this->title = trim($exp[1]);

                                    break;
                                }

			case "php":
			        {
                                    eval($exp[1]); 
                                    break;
                                }

			case "end":
			        {
                                    die($exp[1]);
                                    break;
                                } 
                        
                        case "clear":
                                {
                                    ob_clean();
                                    break;
                                }
		}
		
		
		/* If no command was executed, then the command probably 
		 * didn't exist or there was an error in the line of code.
		 * But, we get rid of the code anyway to prevent it from
		 * showing the HTML source. So we return a null string.
		*/
		
		return "";  
	}
	
	function output($content)
	{
		$lines = explode("\n", $content); 
		$php = false; 
                $code = ""; 

		for ($c = 0; $c < count($lines); $c++)
		{
			if ($lines[$c][0] == Page::COMMAND_SYMBOL)
			{
				$content = str_replace($lines[$c], $this->parse(trim($lines[$c])), $content); 
			}
			else
			{
                            if ($php)
                            {
                                $code .= $lines[$c];
                            }
                            else
                            {
                                if (strlen($lines[$c]) > 0)
                                {
                                    if (strpos("<?php", $lines[$c]) !== false && !$php)
                                    {
                                        $php = true;

                                        $code .= substr($lines[$c], strpos("<?php", $lines[$c]), strlen($lines[$c]) - 1);
                                    }
                                    else if (strpos("?>", $lines[$c]) !== false && $php)
                                    {
                                        $php = false;
                                        
                                        eval($code); 
                                        
                                        $code = "";
                                    }
                                }
                            }
			}
		}

		return $content;
	}
	
	function get_contents($file)
	{
            if (file_exists($file))
            {
                $file = fopen($file, "r");
                $content = "";
                
                while (!feof($file))
                {
                    $content .= fgets($file);
                }
                
                fclose($file);
                
                return $content;
            }

            return "";
        }

	function load($page)
	{
		if (file_exists(PAGE . __f($page) . ".php"))
		{
                    ob_start(); 

                    $page_content = $this->output($this->get_contents(PAGE . __f($page) . ".php"));

                    if ($this->title != Page::DEFAULT_TITLE)
                    {
                        $page_content = str_replace(array("<title>", "</title>"), array("<!--[[", "]]-->"), $page_content);
                        $page_content = str_replace("<!--[[", "<title>" . __s($this->title) . "</title><!--[[", $page_content);
                    }

                    echo $page_content;
		}
		else
		{
                    $this->load(Page::ERROR_PAGE);
		}
	}
}

?>

And when I want to access a page, I send a request like this:


http://localhost/index.php?page=home

It seems to work pretty well. Snippets act as the core files (the files that are included in every page). And you can create commands if you start them with @, like this:

[source lang=cpp]
// example, output Hello World

@ { output : Hello World }

The PHP scripts parses code with that syntax in the page and snippet files, then returns the output. It's actually quite simple. So how is the code? Is it good? By the way, I didn't include all of the code files because there are a lot of them.

Here's a few things you might want to address:

if (file_exists(PAGE . __f($page) . ".php"))

file_exists() is good, but is_readable() is more important due to file access permissions. Use that instead.

$page_content = str_replace(array("", ""), array(""), $page_content);
$page_content = str_replace("< !--[[", "" . __s($this->title) . "< !--[[", $page_content);

I don't get what this does. What's wrong with?:

$content = str_replace('',"<title>$title"); function get_contents($file) {...} You can condense this a LOT. You don't need a separate function. Just use: if ( is_readable($file) ) { $guts = file_get_contents($file); // alternatively, use file() if you need an array of lines } for ($c = 0; $c < count($lines); $c++) this would be cleaner as : foreach ( $lines as $line ) it also saves you some speed because you are not constantly indexing an array. $line = trim(str_replace(array(Page::COMMAND_SYMBOL, "{", "}"), "", $line)); There's no provision for escaping these characters. This will brake as soon as someone wants "{" or "}" in the actual content. Or did i miss something? That said, if your goal is is to create a templating system, i really recommend you check out <a href="http://www.smarty.net/">Smarty</a> if you haven't already. It's easy to use and very intuitive (usually). They've spent a lot of time adding cool features to it. It's worth 2 minutes of your investigational time.

Topic Locked