Updated template class for CMS

I'm not doing too well in school, so to pass my final year, and get my degree I need to do an off-school project. The teacher chose to give me the assignment of an A grade CMS, since I've always had a thing with PHP.

Now I've made a new template class for in this particular CMS, but I'm not completely sure about it, so I came here for your help. Please review my code.

• Have I used things in a correct way?

• Have I used things in a efficient way?

• Is there a better way to achieve the same result?

```
class template
{

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

public function pageExist( $id )
{
$query = $this->db->connection->prepare( 'SELECT id FROM ' . $this->functions->prefix( 'pages' ) . ' where id = ?' );
$query->bind_param( 'i', $id );
$query->execute();
$query->store_result();
if ( $query->num_rows > 0 )
{
return true;
}
else
{
return false;
}
$query->close();
}

public function fetchPage( $id )
{
$query = $this->db->connection->prepare( 'SELECT * FROM ' . $this->functions->prefix( 'pages' ) . ' WHERE id = ?' );
$query->bind_param( 'i', $id );
$query->execute();
$result = $query->get_result();
while ( $row = $result->fetch_assoc() )
{
$this->setTemplateData( 'title', $num_rows[ 'title' ] );

}
$query->close();
}

public function getTheme()
{
$theme = 'theme';
$query = $this->db->connection->prepare( 'SELECT value FROM ' . $this->functions->prefix( 'settings' ) . ' WHERE name = ?' );
$query->bind_param( 's', $theme );
$query->execute();
if ( ! $query )
{
throw new Exception( $query->error(), 1 );
}
$query->bind_result( $result );
while ( $query->fetch() )
{

The class name, why template? Template is much better and follows the naming convention in all sane languages (oook, i know it's PHP :()

Where are $db and $functions? You missed to declare them in the class

class template
{
   private $db;
   private $functions = [];

Your

$query = $this->db->connection->prepare( 'SELECT id FROM ' . $this->functions->prefix( 'pages' ) . ' where id = ?' );

You use prepared statement for where id = ? but now for $this->functions->prefix( 'pages' ) it makes the prepared statement useless.

The same for fetchPage, getTheme.

As user45891 said, your $query->close() will never be executed due to the return. You can improve this with:

$query->close();
return $query->numRows > 0;

This block

if ( ! $query )
{
    throw new Exception( $query->error(), 1 );
}

Great, i love Exceptions but what about a more specific exception? When i write PHP code i check this list to know which exceptions php have. What about UnexpectedValueException or LogicException. LogicException should be used when the error in your code, if $query is false it means you DB is OFF or your question is wrong so the problem should be in your code.

Anyway, your check should go before you use $query! because it will fails with

$query->bind_param( 's', $theme );

while ( $query->fetch() )
{
   return $result;
}

You need only the first value? Well, what about

$query->fetch();
return $result;

?

Anyway, you don't close $query in every function.

Your handlePageLoad don't return everytime, only if it joins $this->pageExist( $id ) path. Maybe you want to add return false;?

You could improve the this code with this:

public function handlePageLoad( $id )
{
    if ( $this->pageExist( $id ) )
    {
        $this->fetchPage( $id );
        $this->loadPage();
        $this->parseParameters();
        return true;
    }

    $this->fetchPage( 404 );
    $this->loadPage();
    $this->parseParameters();
    //header('Location: index.php?page=404');
    echo '!  He\'s a true genius.';
    return false;
}

The indentation of parseParameters is not coerent with the other code (and ugly :()

As i said above, you missed to declare fields in your class. Do it! You will need to remember what fields you use and what fields your class have (and your IDE will help you more!)

How you handle the case where someone uses the same name for lang and template data?

Your parseParameters feels to be very slow with many arguments... what about RegEx?

Why you use three different arrays parameter, langParameters and templateData (except this which is filled by another method) ? They do the same thing? What about only one array?

You need it separate?

I don't use PHP everyday (only when i need to) so i'm not sure, so don't take it serious:

In getTheme you used $theme = 'theme'; because bind_param wants references ok, but what about &"theme"? I'm not sure, but you could try

Anyway, good luck!

StackExchange Code Review Q#60122, answer score: 2