3
\$\begingroup\$

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

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

3. __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->locale_filter_exists($function)){
        return $this->locale_filter_exec($function,$args);
    }
    throw new \exception('RedbeanFVM ::  Method `'.$function.'` doesn\'t exist!');
}

4. snake_case()

Redbean likes for all columns to be snake cased. Rather than rely on Redbean to do this for me, I wanted to be able to do it in a predictable way. I think I was just so far down the rabbit hole when I wrote this, I'm not even sure how it works.

/**
 * Convert a key to snake casing before setting it to the bean.
 * @param string $property the key.
 * @return string
 */
private function snake_case($key)
{
    return strtolower(trim(preg_replace("/(_)\\1+/", "$1",preg_replace('/([^a-zA-Z_])/','_',$key)),'_'));
}

5. configure()

Being a singleton, the library comes preconfigured. To override the settings, you must use configure().

/**
 * configure multiple configuration settings
 * @param Array $c
 */
public static function configure($c)
{
    if(!is_array($c)){
        throw new \exception('RedBeanFVM :: configureAll() expects an array! `'.gettype($c).'` given.');
    }
    foreach($c as $k => $v){
        if(isset(self::$config[$k])){
            self::$config[$k] = $v;
        }else{
            throw new \exception('RedBeanFVM :: configure() `'.$k.'` is not a valid configuration option.');
        }
    }
    //if settings changed on an instantiated instance, we must reinstantiate.
    if(!is_null(self::$instance)){ 
        self::destroyInstance(); 
        self::getInstance();
    }
}

6. __construct()

/**
 * Protected Ctor
 */
protected function __construct()
{
    $c = self::$config;
    $locale = '\\RedBeanFVM\\Locale\\'.$c['locale'];
    self::$locale_filters = new $locale();
}

Now, it gets a bit more complex, all filters are chainable. The desired use case of the library is as follows:

$bean = R::dispense('user');

$required = [
    'Name'=>'name',
    'Email'=>'email',
    'User_Name'=>['rmnl','az_lower'], //chaining multiple rules
    'Password'=>'password_hash'
];

$optional = [
    'username'=>'min' //min is the minimum validation/filter
];

(\RedbeanFVM\RedbeanFVM::getInstance())->generate_model($bean,$required,$optional,$_POST);

7. generate_model()

/**
 * Generate a RedBean Model
 * @param  RedBean_SimpleModel &$bean An instance of RedBean_SimpleModel
 * @param  Array $required A list of required keys and their rules (key=>rule) OR (key=>[rule1,rule2, etc.])
 * @param  Array $optional A list of optional keys. Exceptions are not thrown for optional keys. default empty array.
 * @param  Array $source An array of data where to look for the keys. Default is post
 * @return void
 */
public function generate_model( &$bean, $required, $optional = [], $source = $_POST)
{
    foreach($required as $k => $v){
        if(!isset($source[$k])){
            throw new \exception('Missing form value: '.ucFirst($k));
        }
        if(is_array($v)){
            $bean->{ $this->snake_case($k) } = $this->chain($v,$source[$k]);
        }else{
            $bean->{ $this->snake_case($k) } = $this->{$v}($source[$k]);
        }
    }
    foreach($optional as $k => $v){
        if(isset($source[$k])){
            if(!empty($source[$k])){
                if(is_array($v)){
                    $bean->{ $this->snake_case($k) } = $this->chain($v,$source[$k]);
                }else{
                    $bean->{ $this->snake_case($k) } = $this->{$v}($source[$k]);
                }
            }
        }
    }
}

8. chain()

/**
 * executes an array of filters on an input and returns the output.
 * @param Array $functions
 * @param mixed $input
 * @return string
 */
public function chain($functions,$input)
{
    foreach($functions as $callable){
        $input = $this->{$callable}($input);
    }
    return $input;
}

9. custom_filter()

Allows user to define custom lambda. I'm a guy of modest knowledge, so I don't know if the checks I give are sufficient enough.

/**
 * creates a named callable and adds it to $custom_filters array. useful for creating custom filters.
 * @param string $name the name to assign to the callable.
 * @param closure $callable the callback function.
 */
public function custom_filter($name,$callable)
{
    if(empty($name)){
        throw new \exception('RedBeanFVM :: custom_filter() An Invalid Name was declared.');
    }
    if(method_exists($this,$name)){
        throw new \exception('RedBeanFVM :: custom_filter() `'.$name.'()` is a built in method of RedBeanFVM and a custom filter of that name may not be declared.');
    }
    if(!is_callable($callable)){
        throw new \exception('RedBeanFVM :: custom_filter() Method `'.$name.'` isn\'t a valid callable!');
    }
    $info = new \ReflectionFunction($callable);
    if( $info->getNumberOfParameters() !== 1 || $info->getNumberOfRequiredParameters() !== 1 ){
        throw new \exception('RedbeanFVM :: custom_filter() Method`'.$name.'` declares an invalid number of arguments! only one argument is allowed!' );
    }
    self::$custom_filters[$name] = $callable;
}
\$\endgroup\$
3
\$\begingroup\$

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 {
                ...
            }
        }
    }
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.