patternphpMinor
Classes and variables in PHP
Viewed 0 times
andphpvariablesclasses
Problem
First of all, I'm trying to learn OOP PHP, so any advise will be great.
My idea is to create some kind of MVC Framework CMS kind of thing, just to learn OOP PHP and MVC well.
Let's say that I've got this code (note: I will separate the code pieces in different files, but this was just for testing):
```
';
$returnVal .= print_r($var, TRUE);
$returnVal .= '';
return $returnVal;
}
class Main_Controller {
protected $_vars = array();
protected static $_instance;
private function __construct() {
}
public static function getInstance() {
if (!isset(self::$_instance)) {
self::$_instance = new self();
}
return self::$_instance;
}
public function & __get($name) {
return $this -> _vars[$name];
}
public function __set($name, $value) {
$this -> _vars[$name] = $value;
}
public function loadDefaultClasses(){
$main = Main_Controller::getInstance();
$main -> config = Config::getInstance();
$main -> sql = SQL::getInstance();
}
}
class Config {
protected $_vars = array();
protected static $_instance;
private function __construct() {
$this-> db = array("localhost");
}
public static function getInstance() {
if (!isset(self::$_instance)) {
self::$_instance = new self();
}
return self::$_instance;
}
public function & __get($name) {
return $this -> _vars[$name];
}
public function __set($name, $value) {
$this -> _vars[$name] = $value;
}
}
class SQL{
protected static $_instance;
private function __construct() {
$main = Main_Controller::getInstance();
echo printExtender($main -> config -> db);
}
public static function getInstance() {
if (!isset(self::$_instance)) {
self::$_instance = new self();
}
return self::$_instance;
}
}
$main = Main_Controller::getInstance();
$main
My idea is to create some kind of MVC Framework CMS kind of thing, just to learn OOP PHP and MVC well.
Let's say that I've got this code (note: I will separate the code pieces in different files, but this was just for testing):
```
';
$returnVal .= print_r($var, TRUE);
$returnVal .= '';
return $returnVal;
}
class Main_Controller {
protected $_vars = array();
protected static $_instance;
private function __construct() {
}
public static function getInstance() {
if (!isset(self::$_instance)) {
self::$_instance = new self();
}
return self::$_instance;
}
public function & __get($name) {
return $this -> _vars[$name];
}
public function __set($name, $value) {
$this -> _vars[$name] = $value;
}
public function loadDefaultClasses(){
$main = Main_Controller::getInstance();
$main -> config = Config::getInstance();
$main -> sql = SQL::getInstance();
}
}
class Config {
protected $_vars = array();
protected static $_instance;
private function __construct() {
$this-> db = array("localhost");
}
public static function getInstance() {
if (!isset(self::$_instance)) {
self::$_instance = new self();
}
return self::$_instance;
}
public function & __get($name) {
return $this -> _vars[$name];
}
public function __set($name, $value) {
$this -> _vars[$name] = $value;
}
}
class SQL{
protected static $_instance;
private function __construct() {
$main = Main_Controller::getInstance();
echo printExtender($main -> config -> db);
}
public static function getInstance() {
if (!isset(self::$_instance)) {
self::$_instance = new self();
}
return self::$_instance;
}
}
$main = Main_Controller::getInstance();
$main
Solution
Overview
This isn't OOP. To me, object-oriented programming requires thinking about your programs in terms of classes, their responsibilities, their attributes, and behaviours. This means:
Config Class
Dearth of source code comments aside, let's look at the
In this case, the
The name "Config" indicates that the class is responsible for some kind of configuration. (Note: I absolutely abhor abbreviations.) I would further imagine that the configuration is specifically for the CMS application. So I would have named the class
What can an application configuration class do? I imagine a general configuration class can do the following:
That's its behaviour. The single responsibility is clear: the
There seems to be a trend of not declaring attributes, but using magical
You could, for example, have
SQL Class
Judging by its name, the SQL Class seems to be some kind of holder for SQL statements? This will directly violate encapsulation again, as the class that needs to use the SQL will have to call the SQL class to get it. The SQL class should know nothing about the SQL that other classes need to perform.
I actually go a bit further than this. I don't like exposing SQL in my applications at all. SQL changes a lot over the lifetime of a project. When you have many different fingers all creating their own SQL statements, you wind up with a mess: duplicate code, inefficient queries, a burdened query cache, brittle code that will break anytime a table changes, and no clean way to unit test the queries.
The problem, in my mind, is that there is no contract prototype between a CRUD (Create, Read, Update, or Delete) statement and the calling code. This means that the two cannot easily vary independently.
To resolve this, I move the SQL code into stored procedures and stored functions (collectively stored routines). Some people will despise this approach, others will laud it. I prefer it because it means I can create a contract between the stored routine and the underlying code. (It also means that if I wanted to switch from PHP to another language, there's less code to change.)
Once all the SQL code is written in stored routines, it becomes trivial to write a generic database class that can call those routines with arbitrary parameters.
```
* $db = Database::get();
* $db->call( ... );
*
*/
class Database {
private static $instance;
private $dataStore;
/**
* Sets the connection that this class uses for database transactions.
*/
public function __construct() {
global $dbhost;
global $dbname;
g
This isn't OOP. To me, object-oriented programming requires thinking about your programs in terms of classes, their responsibilities, their attributes, and behaviours. This means:
- Information hiding
- Encapsulation
- Polymorphism
- Collaboration
Config Class
Dearth of source code comments aside, let's look at the
Config class. Here are some questions about the configuration options:- What are the options?
- What do each of the options do?
- How do the options affect the application?
- How are the options communicated to the end user?
In this case, the
Config "class" as written is nothing more than a functional structure (i.e., it is a dynamic array) wrapped in the syntax of a PHP class. A class, by definition, allows instances to have state and behaviour. The Config class has no discernible behaviour. To give it behaviour, we'd have to contemplate the purpose of the class: its raison d'être.The name "Config" indicates that the class is responsible for some kind of configuration. (Note: I absolutely abhor abbreviations.) I would further imagine that the configuration is specifically for the CMS application. So I would have named the class
CMSConfiguration, which inherits from a more generic Configuration class. Let's look at that generic class.What can an application configuration class do? I imagine a general configuration class can do the following:
- Load options (from a stream, a file, command line switches)
- Save options (to maintain current state, or save preferences)
- Answer whether an option is set
- Answer whether an option is equal to a given value
- Export help for the options in an display-device-neutral markup language
That's its behaviour. The single responsibility is clear: the
Configuration class loads and queries the state of configuration options. The implementation might resemble:call( "get_configuration_options" );
// Set the configuration options using the result set.
}
/** Saves the current state of the configuration options. */
public function save() {
}
/**
* Returns true to indicate that the option with the given name is set.
*
* @param $option The name of the option to check.
* @return true The option has a value that is not false.
*/
public function isOptionSet( $option ) {
return $this->getOption( $option );
}
/** ... */
public function isOptionEqual( $option, $value ) {
return $this->getOption( $option ) === $value;
}
/** Writes the help for all the options as an XML document. */
public function exportHelp() {
foreach( $this->getOptions() as $option ) {
// ...
}
}
}
?>There seems to be a trend of not declaring attributes, but using magical
__set and __get functions combined with dynamic arrays. This will lead to unmaintainable code. If the variables that are used by a class are not defined in a single location, how do you expect anyone else to understand how the code works?You could, for example, have
ClassA and ClassB. What is stopping ClassB from using ClassA as its container for attribute values? Nothing. And that's a terrible, terrible thing, as it completely violates both information hiding and encapsulation.SQL Class
Judging by its name, the SQL Class seems to be some kind of holder for SQL statements? This will directly violate encapsulation again, as the class that needs to use the SQL will have to call the SQL class to get it. The SQL class should know nothing about the SQL that other classes need to perform.
I actually go a bit further than this. I don't like exposing SQL in my applications at all. SQL changes a lot over the lifetime of a project. When you have many different fingers all creating their own SQL statements, you wind up with a mess: duplicate code, inefficient queries, a burdened query cache, brittle code that will break anytime a table changes, and no clean way to unit test the queries.
The problem, in my mind, is that there is no contract prototype between a CRUD (Create, Read, Update, or Delete) statement and the calling code. This means that the two cannot easily vary independently.
To resolve this, I move the SQL code into stored procedures and stored functions (collectively stored routines). Some people will despise this approach, others will laud it. I prefer it because it means I can create a contract between the stored routine and the underlying code. (It also means that if I wanted to switch from PHP to another language, there's less code to change.)
Once all the SQL code is written in stored routines, it becomes trivial to write a generic database class that can call those routines with arbitrary parameters.
```
* $db = Database::get();
* $db->call( ... );
*
*/
class Database {
private static $instance;
private $dataStore;
/**
* Sets the connection that this class uses for database transactions.
*/
public function __construct() {
global $dbhost;
global $dbname;
g
Code Snippets
<?php
/**
* Responsible for loading and saving configuraiton options.
*/
class Configuration {
/**
* Loads the configuration options from application's database connection.
*/
public function load() {
Database database = Database::getInstance();
$result = database->call( "get_configuration_options" );
// Set the configuration options using the result set.
}
/** Saves the current state of the configuration options. */
public function save() {
}
/**
* Returns true to indicate that the option with the given name is set.
*
* @param $option The name of the option to check.
* @return true The option has a value that is not false.
*/
public function isOptionSet( $option ) {
return $this->getOption( $option );
}
/** ... */
public function isOptionEqual( $option, $value ) {
return $this->getOption( $option ) === $value;
}
/** Writes the help for all the options as an XML document. */
public function exportHelp() {
foreach( $this->getOptions() as $option ) {
// ...
}
}
}
?><?php
use PDO;
use PDOException;
/**
* Used for interacting with the database. Usage:
* <pre>
* $db = Database::get();
* $db->call( ... );
* </pre>
*/
class Database {
private static $instance;
private $dataStore;
/**
* Sets the connection that this class uses for database transactions.
*/
public function __construct() {
global $dbhost;
global $dbname;
global $dbuser;
global $dbpass;
try {
$this->setDataStore(
new PDO( "pgsql:dbname=$dbname;host=$dbhost", $dbuser, $dbpass ) );
}
catch( PDOException $ex ) {
$this->log( $ex->getMessage() );
}
}
/**
* Returns the singleton database instance.
*/
public function get() {
if( self::$instance === null ) {
self::$instance = new Database();
}
return self::$instance;
}
/**
* Call a database function and return the results. If there are
* multiple columns to return, then the value for $params must contain
* a comma; otherwise, without a comma, the value for $params is used
* as the return column name. For example:
*
*- SELECT $params FROM $proc( ?, ? ); -- with comma
*- SELECT $proc( ?, ? ) AS $params; -- without comma
*- SELECT $proc( ?, ? ); -- empty
*
* @param $proc Name of the function or stored procedure to call.
* @param $params Name of parameters to use as return columns.
*/
public function call( $proc, $params = "" ) {
$args = array();
$count = 0;
$placeholders = "";
// Key is zero-based (e.g., $proc = 0, $params = 1).
foreach( func_get_args() as $key => $parameter ) {
// Skip the $proc and $params arguments to this method.
if( $key < 2 ) continue;
$count++;
$placeholders = empty( $placeholders ) ? "?" : "$placeholders,?";
array_push( $args, $parameter );
}
$sql = "";
if( empty( $params ) ) {
// If there are no parameters, then just make a call.
$sql = "SELECT $proc( $placeholders )";
}
else if( strpos( $params, "," ) !== false ) {
// If there is a comma, select the column names.
$sql = "SELECT $params FROM $proc( $placeholders )";
}
else {
// Otherwise, select the result into the given column name.
$sql = "SELECT $proc( $placeholders ) AS $params";
}
$db = $this->getDataStore();
$statement = $db->prepare( $sql );
for( $i = 1; $i <= $count; $i++ ) {
$statement->bindParam( $i, $args[$i - 1] );
}
try {
$result = null;
if( $statement->execute() === true ) {
$result = $statement->fetchAll( PDO::FETCH_ASSOC );
$this->decodeArray( $result );
}
else {
// \todo Send an e-mail.
$info = $statement->errorInfo();
$this->log( "SQL failed: $sql" );
$this->log( "Error: ". $info[2] );
}
}
catch( PDOException $ex ) {
// \todo Send an e-mail.
$this->log( $ex->getMessage() );
}
return $result;
}
/**
* Converts// Returns a boolean value
$result = $db->call( "is_existing_cookie", "existing", $cookie_value );
// Calls a stored procedure
$this->call( "procedure_insert", "", $this->getId(), $action_name_id );
// Returns a tuple pair of ids and labels
return $this->call( "get_list", "id, label", $this->getId() );return $this->call( "get_list", "id, label", $this->getId() );try {
$result = $this->call( ... );
}
catch( Exception $e ) {
$result = $this->getErrorXML();
}Context
StackExchange Code Review Q#27564, answer score: 6
Revisions (0)
No revisions yet.