HiveBrain v1.2.0
Get Started
← Back to all entries
snippetphpMinor

Filter, Validate, and Generate Redbean models programmatically

Submitted by: @import:stackexchange-codereview··
0
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:

  1. 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;
    }
}


  1. 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);
}


  1. __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:

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.