From ecf86b9818ac9c372370e8396781c13d2db458b4 Mon Sep 17 00:00:00 2001 From: Josh Sherman Date: Fri, 10 Oct 2008 01:56:17 +0000 Subject: [PATCH] Removed the Singleton class, removed the ImageUtils class (for now) and switched the DB class to be an instantiated object and not a Singleton. git-svn-id: http://svn.cleancode.org/svn/pickles@67 4d10bc64-7434-11dc-a737-d2d0f8310089 --- classes/Config.php | 25 ++++++---- classes/Controller.php | 12 +++-- classes/DB.php | 61 +++++++--------------- classes/ImageUtils.php | 111 ----------------------------------------- classes/Model.php | 6 +-- classes/Singleton.php | 97 ----------------------------------- classes/Viewer.php | 4 +- 7 files changed, 47 insertions(+), 269 deletions(-) delete mode 100755 classes/ImageUtils.php delete mode 100644 classes/Singleton.php diff --git a/classes/Config.php b/classes/Config.php index aee4de3..1f404b1 100755 --- a/classes/Config.php +++ b/classes/Config.php @@ -33,7 +33,7 @@ * @usage $config = Config::getInstance(); *$config->node; // Returns SimpleXML object */ -class Config extends Singleton { +class Config { /** * Private instance of the Config class @@ -51,18 +51,24 @@ class Config extends Singleton { private function __construct() { } /** - * Gets an instance of the configuration object + * __clone + */ + public function __clone() { + trigger_error('Cloning is not available on a Singleton (that would defeat the purpose wouldn\'t it?)', E_USER_ERROR); + } + + /** + * Gets an instance of the requested class object * - * Determines if a Config object has already been instantiated, - * if so it will use it. If not, it will create one. + * Determines if a requested class object has already + * been instantiated, if so it will use it. If not, + * it will create one. * - * @return An instace of the Config class + * @return An instace of the requested class */ public static function getInstance() { - $class = __CLASS__; - - if (!self::$_instance instanceof $class) { - self::$_instance = new $class(); + if (!self::$_instance instanceof Config) { + self::$_instance = new Config(); } return self::$_instance; @@ -79,6 +85,7 @@ class Config extends Singleton { * @todo Add the ability to load in multiple configuration files. */ public static function load($file) { + $config = Config::getInstance(); if (file_exists($file)) { diff --git a/classes/Controller.php b/classes/Controller.php index 7e24495..ccc6095 100755 --- a/classes/Controller.php +++ b/classes/Controller.php @@ -48,6 +48,8 @@ class Controller extends Object { // Load the config for the site passed in $config = Config::getInstance(); + $db = new DB($config->database->hostname, $config->database->username, $config->database->password, $config->database->database); + // Generate a generic "site down" message if ($config->getDisabled()) { exit("

{$_SERVER['SERVER_NAME']} is currently down for maintenance

"); @@ -91,25 +93,25 @@ class Controller extends Object { require_once $model_file; if (class_exists($class)) { - $model = new $class; + $model = new $class($config, $db); } } // Tries to load the shared model else if (file_exists($shared_model_file) && $shared_model_name != false) { if (strpos($shared_model_name, '/') === false) { - $class = $shared_model_name; + $class = $shared_model_name; } else { $class = str_replace('/', '_', $shared_model_name); } if (class_exists($class)) { - $model = new $class; + $model = new $class($config, $db); } } // Loads the stock model else { - $model = new Model(); + $model = new Model($config, $db); } // Checks if we loaded a model file and no class was present @@ -133,7 +135,7 @@ class Controller extends Object { } // Creates a new viewer object - $viewer = Viewer::factory($model->getViewer()); + $viewer = Viewer::create($model->getViewer()); // Sets the viewers properties $viewer->model_name = $model_name; diff --git a/classes/DB.php b/classes/DB.php index 7f55b69..677527b 100755 --- a/classes/DB.php +++ b/classes/DB.php @@ -40,12 +40,12 @@ * crutch for actually being able to write SQL. * @todo Rename me to Database (maybe) */ -class DB extends Singleton { +class DB extends Object { - /** - * Private instance of the DB class - */ - private static $instance; + private $hostname; + private $username; + private $password; + private $database; /** * Private MySQL resources @@ -53,27 +53,11 @@ class DB extends Singleton { private $connection; private $results; - /** - * Private constructor - */ - private function __construct() { } - - /** - * Gets an instance of the database object - * - * Determines if a DB object has already been instantiated, if so it - * will use it. If not, it will create one. - * - * @return object An instace of the DB class - */ - public static function getInstance() { - $class = __CLASS__; - - if (!self::$instance instanceof $class) { - self::$instance = new $class(); - } - - return self::$instance; + public function __construct($hostname, $username, $password, $database) { + $this->hostname = $hostname; + $this->username = $username; + $this->password = $password; + $this->database = $database; } /** @@ -83,25 +67,22 @@ class DB extends Singleton { * configuration options that are available in the Config object. * * @return boolean Based on the success or failure of mysql_connect() - * @todo Remove the error supressing @ from mysql_connect() */ public function open() { if (!is_resource($this->connection)) { - $config = Config::getInstance(); - - if ($config->database->hostname == '') { - $config->database->hostname = 'localhost'; + if ($this->hostname == '') { + $this->hostname = 'localhost'; } - if (isset($config->database->username, $config->database->password, $config->database->database)) { + if (isset($this->username, $this->password, $this->database)) { /** * @todo I removed the @ and changed to be pconnect... let's see */ - $this->connection = mysql_pconnect($config->database->hostname, $config->database->username, $config->database->password); + $this->connection = mysql_pconnect($this->hostname, $this->username, $this->password); if (is_resource($this->connection)) { - if (!mysql_select_db($config->database->database, $this->connection)) { - Error::addWarning("There was an error selecting the '" . $this->database->database , "' database"); + if (!mysql_select_db($this->database, $this->connection)) { + Error::addWarning("There was an error selecting the '" . $this->database , "' database"); return false; } else { @@ -148,13 +129,12 @@ class DB extends Singleton { * * @param string $sql SQL statement to be executed * @return boolean Returns the status of the execution - * @todo Need to get rid of the error suppressing @ symbol. */ public function execute($sql) { $this->open(); if (trim($sql) != '') { - $this->results = @mysql_query($sql, $this->connection); + $this->results = mysql_query($sql, $this->connection); if (empty($this->results)) { Error::addError('There was an error executing the SQL'); Error::addError(mysql_error()); @@ -180,7 +160,6 @@ class DB extends Singleton { * * @param string $sql SQL statement to be executed (optional) * @return string Returns the value of the field or null if none - * @todo Need to remove the error supression * @todo Right now it assumes your query only returns a single field, * that probably should be changed to allow someone to specify * what field they want from a row of data. Actually, this is @@ -196,7 +175,7 @@ class DB extends Singleton { } if (is_resource($this->results)) { - $results = @mysql_fetch_row($this->results); + $results = mysql_fetch_row($this->results); if (is_array($results)) { return $results[0]; } @@ -221,7 +200,6 @@ class DB extends Singleton { * * @param string $sql SQL statement to be executed (optional) * @return mixed The row in an associative array or null if none - * @todo Need to remove the error supression * @todo Right now it assumes your query only returns a single row, * that probably should be changed to allow someone to specify * what row they want from a set of data. Actually, this is @@ -242,7 +220,7 @@ class DB extends Singleton { } if (is_resource($this->results)) { - $results = @mysql_fetch_assoc($this->results); + $results = mysql_fetch_assoc($this->results); if (is_array($results)) { return $results; } @@ -265,7 +243,6 @@ class DB extends Singleton { * * @param string $sql SQL statement to be executed (optional) * @return string Returns the rows in an array or null if none - * @todo Need to remove the error supression * @todo Another debate point, should it return false instead of null, * or perhaps have some sort of error indicator in the result * set? diff --git a/classes/ImageUtils.php b/classes/ImageUtils.php deleted file mode 100755 index 6d7c6f0..0000000 --- a/classes/ImageUtils.php +++ /dev/null @@ -1,111 +0,0 @@ -. - * - * @author Joshua John Sherman - * @copyright Copyright 2007, 2008 Joshua John Sherman - * @link http://phpwithpickles.org - * @license http://www.gnu.org/copyleft/lesser.html - * @package PICKLES - */ - -/** - * Incomplete collection of image manipulation utilities - * - * I'm not entirely sure if this class is being utilized anywhere, especially - * since a lot of the functions are not finished, or var_dump() stuff. The - * functions are (or eventually will be) wrappers for ImageMagick commands. The - * other route would be to code it all in pure PHP as to not have to add another - * dependency to PICKLES. After checking the PHP documentation on image - * processing, it seems there's an ImageMagick module that can utilized, that may - * end up being the route that is taken. Until then, this class is useless shit. - * - * @todo Make the class usable. - */ -class ImageUtils { - - /** - * Checks an image format - * - * This is basically just a wrapper for in_array(). The passed image type is - * checked against the passed array of types (else it will use the default - * list). - * - * @param string $type The type of being being checked - * @param array $types An array of valid image types, defaults to the common - * web formats of jpg, gif and png - * @return boolean If the passed type is in the list of valid types - */ - static function check($type, $types = array('image/jpeg', 'image/gif', 'image/png')) { - if (!is_array($types)) { - $types[0] = $types; - } - - return in_array($type, $types); - } - - /** - * Moves an uploaded file - * - * Handles not only moving an uploaded file to another location, but removes - * the original file once it's been moved. It's ashame that the function - * move_uploaded_file() just copies a file, and doesn't actually move it. - * - * @param string $origin The uploaded file that is to be moved - * @param string $destination The destination for the origin file - * @todo This function needs to return the status of the move - * @todo Currently if the move fails, the origin file will still be removed. - */ - static function move($origin, $destination) { - move_uploaded_file($origin, $destination); - imagedestroy($origin); - } - - static function resize() { - - } - - static function convert($original, $destination, $keep_original = true) { - var_dump('convert ' . $original . ' ' . $destination); - - var_dump( exec('convert ' . $original . ' ' . $destination) ); - } - - /* - - - if ($_FILES['image']['type'] == 'image/jpeg') { - $original = $directory . 'original.jpg'; - - $source = imagecreatefromjpeg($original); - list($width, $height) = getimagesize($original); - - $sizes = array('small' => 75, 'medium' => 150, 'large' => 500); - foreach ($sizes as $name => $size) { - $temp = imagecreatetruecolor($size, $size); - imagecopyresampled($temp, $source, 0, 0, 0, 0, $size, $size, $width, $height); - imagejpeg($temp, "{$directory}{$name}.jpg", 85); - - imagedestroy($temp); - } - - imagedestroy($source); - */ -} - -?> diff --git a/classes/Model.php b/classes/Model.php index 695d2a9..e33dcb0 100644 --- a/classes/Model.php +++ b/classes/Model.php @@ -64,11 +64,11 @@ class Model extends Object { * Handles calling the parent constructor and sets up the model's * internal config and database object */ - public function __construct() { + public function __construct($config, $db) { parent::__construct(); - $this->config = Config::getInstance(); - $this->db = DB::getInstance(); + $this->config = $config; + $this->db = $db; } /** diff --git a/classes/Singleton.php b/classes/Singleton.php deleted file mode 100644 index 55d9e91..0000000 --- a/classes/Singleton.php +++ /dev/null @@ -1,97 +0,0 @@ -. - * - * @author Joshua John Sherman - * @copyright Copyright 2007, 2008 Joshua John Sherman - * @link http://phpwithpickles.org - * @license http://www.gnu.org/copyleft/lesser.html - * @package PICKLES - */ - -/** - * Singleton Class - * - * This is the file that you include on the page you're instantiating the - * controller from (typically index.php). The path to the PICKLES code base - * is established as well as the path that Smarty will use to store the - * compiled pages. - */ -class Singleton { - - /** - * Protected collection of data - */ - protected $data; - - /** - * Private constructor - */ - private function __construct() { } - - /** - * __clone - */ - public function __clone() { - trigger_error('Cloning is not available on a Singleton (that would defeat the purpose wouldn\'t it?)', E_USER_ERROR); - } - - /** - * Gets a variable - * - * @param string $variable Name of the variable to be returned - * @param string $array_element Name of the array element that's part - * of the requested variable (optional) - * @return Returns either the variable value or false if no variable. - * @todo Need better checking if the passed variable is an array when - * the array element value is present - * @todo Returning false could be misleading, especially if you're - * expecting a boolean value to begin with. Perhaps an error - * should be thrown? - */ - /* - public function get($variable, $array_element = null) { - if (isset($this->data[$variable])) { - if (isset($array_element)) { - $array = $this->data[$variable]; - - if (isset($array[$array_element])) { - return $array[$array_element]; - } - } - else { - return $this->data[$variable]; - } - } - - return false; - } - */ - - /** - * Sets a variable - * - * @param string $variable Name of the variable to be set - * @param mixed $value Value to be assigned to the passed variable - */ - //public function set($variable, $value) { - // $this->data[$variable] = $value; - //} -} - -?> diff --git a/classes/Viewer.php b/classes/Viewer.php index b57bc75..c86b369 100644 --- a/classes/Viewer.php +++ b/classes/Viewer.php @@ -38,7 +38,7 @@ class Viewer { private function __construct() { } /** - * Viewer factory + * Viewer Factory * * Creates an instance of the Viewer type that the model requests. * @@ -48,7 +48,7 @@ class Viewer { * potentially easier to reference from the model (since the * constants would each be uppercase instead of mixedcase. */ - public static function factory($type) { + public static function create($type) { $class = 'Viewer_' . $type; return new $class(); }