views:

207

answers:

2

I have been looking into Dependency Injection.

  • Am I on to something or is it completely off?
  • Is the code good or bad - Dependency Injection or not?

The below code is the foundation for a CMS system

Right now there is a table called "page_details" with all the web pages stored in it.

Directory/file structure

.htaccess
index.php
classes/Db.class.php
classes/Page.class.php
config/config.php
config/init.php

.htaccess

# Mod rewrite enabled.
Options +FollowSymLinks
RewriteEngine on

# ---- Rules ----

RewriteRule ^([A-Za-z0-9-_]+)\.html$ index.php?page=$1 [NC,L]

index.php

<?php require_once ('config/init.php'); ?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd"&gt;
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
    <meta http-equiv="Content-type" content="text/html; charset=iso-8859-1" />
    <meta http-equiv="imagetoolbar" content="no" />
    <title></title>
    <meta name="Description" content="" />
    <meta name="Keywords" content="" />
    <link href="/css/styles.css" media="screen" rel="Stylesheet" type="text/css" />
</head>
<body>
<?php
$page = new Pages($db);
print_r($page->get_page($_GET['page']));
?>
</body>
</html>

Db.class.php

<?php
class Db
{
    private $dbhost;
    private $dbuser;
    private $dbpassword;
    private $dbname;
    private $connection;
    public $query;
    function __construct($dbhost, $dbuser, $dbpassword, $dbname)
    {
        $this->dbhost = $dbhost;
        $this->dbuser = $dbuser;
        $this->dbpassword = $dbpassword;
        $this->dbname = $dbname;
    }
    public function open_connection()
    {
        try
        {
            $this->connection = mysqli_connect($this->dbhost, $this->dbuser, $this->
                dbpassword, $this->dbname);
        }
        catch (exception $e)
        {
            throw $e;
        }
    }
    public function close($query)
    {
        try
        {
            mysqli_close($this->connection);
        }
        catch (exception $e)
        {
            throw $e;
        }
    }
    public function query($query)
    {
        try
        {
            $this->open_connection();
            $result = mysqli_query($this->connection, $query);
            return $result;
        }
        catch (exception $e)
        {
            throw $e;
        }
        $this->close_connection();
    }
    public function fetchArray($query)
    {
        $row = mysqli_fetch_assoc($query);
        return $row;
    }
    public function count_rows($query)
    {
        $row = mysqli_num_rows($query);
        return $row;
    }
    public function rows_affected()
    {
        $row = mysqli_affected_rows($this->connection);
        return $row;
    }
    public function created_id()
    {
        $row = mysqli_insert_id($this->connection);
        return $row;
    }
}
?>

Page.class.php

<?php
class Pages
{
    private $db;
    function __construct($db)
    {
        $this->db = $db;
    }
    function get_page($seo_url)
    {
        $sql = $this->db->query("SELECT * FROM page_details WHERE seo_url='$seo_url'");
        $row = $this->db->fetchArray($sql);
        return $row;
    }
}
?>

config.php

<?php
$config = array();
$config['dbtype'] = 'mysqli';
$config['dbhost'] = 'localhost';
$config['dbname'] = 'name';
$config['dbuser'] = 'user';
$config['dbpassword'] = 'password';
$config['absolute_path'] = '/var/www/vhosts/example.com/httpdocs';
$config['website_root'] = 'http://www.example.com/';
$config['dummy'] = '';
?>

init.php

<?php
require_once ('config/config.php');
function __autoload($class_name)
{
    require_once (''.$config['absolute_path'].'classes/' . $class_name . '.class.php');
}
$db = new Db($config['dbhost'], $config['dbuser'], $config['dbpassword'], $config['dbname']);
?>
+1  A: 

I'm not sure how you expect to get __autoload to be called (I don't see you calling new someclass anywhere in your code), but on the face of it using __autoload to automatically include classes that should be loaded is a good idea, and you're using it right.

Some general comments on your code though:

  1. I would have used PDO instead of mysqli directly - it somewhat safer (SQL injection wise), more future proof, and I think its also easier.
  2. I would check that the required file exists before attempting to require it, and throw a nice exception that can be caught by the application and reported in a nice way.
  3. you probably want to print your content and not print_r it, but I guess you do that for debugging.
Guss
Cudos
A: 

The dependency injection is the DB -> Pages? Yes, looks good. This seems like a reasonable approach.

Here's an encapsulation (not DI) idea: You might think what you are getting out of the Pages class. Right now it is just giving you the name of the db table. What if this was a Page? Instead of returning the $row of data, it could own it. You could then add methods to Page to help access that various columns of the page data. This architecture would give you a place to put any code about the Page. This would be instead of going directly at the $row in the display code (the print_r line now).

ndp
Interesting point. Could you please explain some more on this?
Cudos