snippetphpMinor
Filter, Validate, and Generate Redbean models programmatically
Viewed 0 times
validategeneratefilterandmodelsredbeanprogrammatically
Problem
A while ago, I wrote my first Composer Package. The purpose is to completely avoid the tedious work of filtering and validating user input then constructing the model. I have questions about multiple functions:
During the development process, I was shopping around, looking for instructions on how to write an autoloader. All of the solutions I found seemed overly verbose / complicated / innefficient. So I wrote my own, and it seems to work.
I'm wondering what your thoughts are on it:
I decided the library would be a singleton, to make retrieval of the instance possible regardless of scope.
I wanted to make it easy for users to define their own custom lambda's for cases not covered by the library, like, for example, if you needed to query database, use a special regex, etc.
I also wanted to have "Locale" packs, that is, country specific filters.
Having this in mind, I overloaded call and used it to attempt to call a custom filter or a locale filter, throwing the exception at last resort. I think it's pretty nifty, but you may probably disagree.
```
/**
* This magic method searches the list of user defined filters for a match. if none is found, an exception is raised.
* @param callable $function
* @param mixed $args
* @return mixed
*/
public function __call($function,$args = false)
{
if($this->custom_filter_exists($function)){
return $this->custom_filter_exec($function,$args);
}
if($this->local
autoload(): an Autoloader
During the development process, I was shopping around, looking for instructions on how to write an autoloader. All of the solutions I found seemed overly verbose / complicated / innefficient. So I wrote my own, and it seems to work.
I'm wondering what your thoughts are on it:
/**
* Autoloader
* @param $class
* @return void
*/
public static function autoload($class)
{
$file = __DIR__ . str_replace('\\','/', preg_replace('/'. __NAMESPACE__ .'/','',$class,1)) . '.php';
if(file_exists($file)){
include $file;
}
}getInstance()
I decided the library would be a singleton, to make retrieval of the instance possible regardless of scope.
/**
* Retrieve the Singleton instance of RedBeanFVM
* @return RedBeanFVM
*/
public static function getInstance()
{
return (is_null(self::$instance) ? self::$instance = new self : self::$instance);
}__call()
I wanted to make it easy for users to define their own custom lambda's for cases not covered by the library, like, for example, if you needed to query database, use a special regex, etc.
I also wanted to have "Locale" packs, that is, country specific filters.
Having this in mind, I overloaded call and used it to attempt to call a custom filter or a locale filter, throwing the exception at last resort. I think it's pretty nifty, but you may probably disagree.
```
/**
* This magic method searches the list of user defined filters for a match. if none is found, an exception is raised.
* @param callable $function
* @param mixed $args
* @return mixed
*/
public function __call($function,$args = false)
{
if($this->custom_filter_exists($function)){
return $this->custom_filter_exec($function,$args);
}
if($this->local
Solution
This expression is needlessly too complex:
It replaces non-alphabetic and non-underscore characters with an underscore, and finally trim any messaging and trailing underscores. There are several unnecessary parentheses, and the
In expressions like this, where there is an assignment in both branches of an if-else with a complex expression on the left hand side:
It's better to extract the complex left hand side to avoid code duplication, like this:
Also, this if-else chain appears twice in the code. It would be good to move this code into a helper function to avoid code duplication.
As with any utility library, it should be spot-on, and that includes formatting too. I recommend adding more spaces around the parentheses and cuddly braces in your if-else chains. For example instead of this:
Format like this, which is slightly easier to read:
return strtolower(trim(preg_replace("/(_)\\1+/", "$1",preg_replace('/([^a-zA-Z_])/','_',$key)),'_'));It replaces non-alphabetic and non-underscore characters with an underscore, and finally trim any messaging and trailing underscores. There are several unnecessary parentheses, and the
\\1 back reference is an unnecessary complication too. A simpler way to achieve the same thing:return strtolower(trim(preg_replace("/__+/", "_", preg_replace("/[^a-zA-Z_]/", "_", $key)), "_"));In expressions like this, where there is an assignment in both branches of an if-else with a complex expression on the left hand side:
if(is_array($v)){
$bean->{ $this->snake_case($k) } = $this->chain($v,$source[$k]);
}else{
$bean->{ $this->snake_case($k) } = $this->{$v}($source[$k]);
}It's better to extract the complex left hand side to avoid code duplication, like this:
if(is_array($v)){
$value = $this->chain($v,$source[$k]);
}else{
$value = $this->{$v}($source[$k]);
}
$bean->{ $this->snake_case($k) } = $value;Also, this if-else chain appears twice in the code. It would be good to move this code into a helper function to avoid code duplication.
As with any utility library, it should be spot-on, and that includes formatting too. I recommend adding more spaces around the parentheses and cuddly braces in your if-else chains. For example instead of this:
if(isset($source[$k])){
if(!empty($source[$k])){
if(is_array($v)){
...
}else{
...
}
}
}Format like this, which is slightly easier to read:
if (isset($source[$k])) {
if (!empty($source[$k])) {
if (is_array($v)) {
...
} else {
...
}
}
}Code Snippets
return strtolower(trim(preg_replace("/(_)\\1+/", "$1",preg_replace('/([^a-zA-Z_])/','_',$key)),'_'));return strtolower(trim(preg_replace("/__+/", "_", preg_replace("/[^a-zA-Z_]/", "_", $key)), "_"));if(is_array($v)){
$bean->{ $this->snake_case($k) } = $this->chain($v,$source[$k]);
}else{
$bean->{ $this->snake_case($k) } = $this->{$v}($source[$k]);
}if(is_array($v)){
$value = $this->chain($v,$source[$k]);
}else{
$value = $this->{$v}($source[$k]);
}
$bean->{ $this->snake_case($k) } = $value;if(isset($source[$k])){
if(!empty($source[$k])){
if(is_array($v)){
...
}else{
...
}
}
}Context
StackExchange Code Review Q#96758, answer score: 3
Revisions (0)
No revisions yet.