patternphpMinor
PHP magic function for accessors and mutators
Viewed 0 times
phpmagicaccessorsfunctionmutatorsforand
Problem
I have implemented the following PHP magic method behavior to save my time of creating accessor and mutators for properties of any class that extends this base class.
I am looking for suggesiongs on improving it for my application that assumes the following:
Properties of the form:
Methods of the form:
I have some points that I think need more research:
-
Is it a secure way? What are the insecurities here?
-
Is my way of handling errors by using
-
Is there a best practice or a design pattern that addresses such a solution for reusability?
-
Am I exaggerating the steps? Are there any steps can be combine into one step with the same results?
The __call function of my Base class:
I am looking for suggesiongs on improving it for my application that assumes the following:
Properties of the form:
$_first_name
$_user_id
Methods of the form:
getUserId()
setFirstName("Mohammad")
I have some points that I think need more research:
-
Is it a secure way? What are the insecurities here?
-
Is my way of handling errors by using
throw new Exception('my message'); a good practice? What are the alternatives?-
Is there a best practice or a design pattern that addresses such a solution for reusability?
-
Am I exaggerating the steps? Are there any steps can be combine into one step with the same results?
The __call function of my Base class:
class Base
{
private $_first_name; // just an example
public function __call($name, $args)
{
// The begining of a method must be in all lower case characters.
// E.g. set, get
$result = preg_match('/^[a-z]*/', $name, $matches);
if($result !== 1)
{
throw new Exception('Action not recognized.');
}
// Hold the found action.
$action = $matches[0];
// Find the rest of the method name.
$result = preg_match_all('/[A-Z][a-z]*/', $name, $matches);
// $matches will hold a multi-dimensional array
// we need the first 1D array only.
$matches = $matches[0];
if($result $property = $args[0];
return;
}
else
{
throw new Exception('You must provide 1 argument only.');
}
}
// Accessor
case 'get':
{
return $this->$property;
}
}
}
}Solution
Exceptions are fine, but consider using less general
First thing that strikes me as odd about your code is that you're using underscore to name your properties, but CamelCase to name your methods. This is quite inconsistent and leads to unnecessary complexity here. Using naming convention that actually helps you is a good idea.
With that in mind, a little change in naming convention could turn into something like this:
Notice i use a an array
Consider this piece of code:
We use our
Now with your code you'll have complications with overriding your setters, and that doesn't make it safe in any way.
Now, reusability. In your base class, you write all your fields explicitly, which doesn't seem very reusable to me. Look this code (warning, large wall of text incoming).
Now, if you want to create a
And use it at your leisure. Later you can add active record methods to
As to why i used array
Exception classes, like BaseException or whatever. This will allow for more flexible error reporting.First thing that strikes me as odd about your code is that you're using underscore to name your properties, but CamelCase to name your methods. This is quite inconsistent and leads to unnecessary complexity here. Using naming convention that actually helps you is a good idea.
With that in mind, a little change in naming convention could turn into something like this:
function __call($method, $args)
{
$action = substr($method, 0, 3);
$prop = strtolower(substr($method, 3));
$getter = "get".ucfirst($prop);
$setter = "set".ucfirst($prop);
switch ($action) {
case "get" :
if (isset($this->_fields[$prop])) {
return $this->_fields[$prop];
} else throw new EntityException("Unknown property {$prop}");
break;
case "set" :
if (isset($this->_fields[$prop])) {
$this->_fields[$prop] = $args[0];
return null;
} else
throw new EntityException("Unknown property {$prop}");
break;
default : throw new EntityException("Unknown action {$action[0]}");
}
}Notice i use a an array
$this->_fields to store all my fields. This goes for my second point - Safety and Reusability.Consider this piece of code:
class User
{
private $_name;
function getName(){
return $this->_name;
}
}We use our
getName method everywhere, but later we need to add another field - second name. We can safely insert this second field inside our getName and the code would change without taking any further actions. This is kinda the whole point behind getters and setters - incapsulating logic away.Now with your code you'll have complications with overriding your setters, and that doesn't make it safe in any way.
Now, reusability. In your base class, you write all your fields explicitly, which doesn't seem very reusable to me. Look this code (warning, large wall of text incoming).
abstract class Entity
{
private $_fields;
protected $schema = array();
function __construct(array $data = array())
{
/** Check that our concrete implementation has its Schema */
if (!$this->schema) {
throw new EntityException("Class ".get_called_class()." Schema not defined");
}
/**
* mapping fields value to our schema
*
* @TODO Move to separate method to do this not only on Construct;
*/
$this->_fields = array_reduce(
$this->schema,
function ($fields, $element) use ($data) {
$fields[$element] = $data[$element];
return $fields;
}
);
}
function __call($method, $args)
{
/** Same call implementation here as above */
}
/**
* Inner getter for getter/setter overrides
*/
protected function getField($name)
{
if (!isset($this->_fields[$name])) {
throw new EntityException ("Property {$name} not found");
}
return $this->_fields[$name];
}
/**
* Inner setter for getter/setter overrides
*/
protected function setField($name, $value)
{
if (!isset($this->_fields[$name])) {
throw new EntityException ("Property {$name} not found");
}
$this->_fields[$name] = $value;
}
}
/**
* Class User
*
* Concrete implementation of Entity class
*
*/
class User extends Entity
{
/**
* List of our entity fields
* @var array
*/
protected $schema = array("id", "name", "email", "phone");
/**
* method getName concrete implementation;
*
* @return string
*/
function getName()
{
return "My name is ".$this->getField("name");
}
}
try {
$userData = array("id" => 1, "name" => "Andrew", "phone" => "000000000", "email" => "test@test.com");
$user = new User($userData);
echo $user->getName()."";
echo $user->getEmail()."";
$user->setPhone("111111111");
echo $user->getPhone();
} catch (Exception $e) {
echo $e->getMessage();
}Now, if you want to create a
News entity, you can simply write:class News extends Entity
{
protected $schema = array("id", "title", "text", "time");
}And use it at your leisure. Later you can add active record methods to
Entity, like save, delete, update and so on, and still use them on any entity you might want.As to why i used array
$_fields instead of simply listing fields - array is much more convenient when you have to map values to your fields and build queries from them.Code Snippets
function __call($method, $args)
{
$action = substr($method, 0, 3);
$prop = strtolower(substr($method, 3));
$getter = "get".ucfirst($prop);
$setter = "set".ucfirst($prop);
switch ($action) {
case "get" :
if (isset($this->_fields[$prop])) {
return $this->_fields[$prop];
} else throw new EntityException("Unknown property {$prop}");
break;
case "set" :
if (isset($this->_fields[$prop])) {
$this->_fields[$prop] = $args[0];
return null;
} else
throw new EntityException("Unknown property {$prop}");
break;
default : throw new EntityException("Unknown action {$action[0]}");
}
}class User
{
private $_name;
function getName(){
return $this->_name;
}
}abstract class Entity
{
private $_fields;
protected $schema = array();
function __construct(array $data = array())
{
/** Check that our concrete implementation has its Schema */
if (!$this->schema) {
throw new EntityException("Class ".get_called_class()." Schema not defined");
}
/**
* mapping fields value to our schema
*
* @TODO Move to separate method to do this not only on Construct;
*/
$this->_fields = array_reduce(
$this->schema,
function ($fields, $element) use ($data) {
$fields[$element] = $data[$element];
return $fields;
}
);
}
function __call($method, $args)
{
/** Same call implementation here as above */
}
/**
* Inner getter for getter/setter overrides
*/
protected function getField($name)
{
if (!isset($this->_fields[$name])) {
throw new EntityException ("Property {$name} not found");
}
return $this->_fields[$name];
}
/**
* Inner setter for getter/setter overrides
*/
protected function setField($name, $value)
{
if (!isset($this->_fields[$name])) {
throw new EntityException ("Property {$name} not found");
}
$this->_fields[$name] = $value;
}
}
/**
* Class User
*
* Concrete implementation of Entity class
*
*/
class User extends Entity
{
/**
* List of our entity fields
* @var array
*/
protected $schema = array("id", "name", "email", "phone");
/**
* method getName concrete implementation;
*
* @return string
*/
function getName()
{
return "My name is ".$this->getField("name");
}
}
try {
$userData = array("id" => 1, "name" => "Andrew", "phone" => "000000000", "email" => "test@test.com");
$user = new User($userData);
echo $user->getName()."<br />";
echo $user->getEmail()."<br />";
$user->setPhone("111111111");
echo $user->getPhone();
} catch (Exception $e) {
echo $e->getMessage();
}class News extends Entity
{
protected $schema = array("id", "title", "text", "time");
}Context
StackExchange Code Review Q#41392, answer score: 5
Revisions (0)
No revisions yet.