[stdlib] Add guard utils and traits #5365

Merged
merged 1 commit into from Jan 3, 2014

Projects

None yet

6 participants

@texdc
Contributor
texdc commented Oct 27, 2013

The check for array or Traversable code is littered throughout the framework. This is a prime opportunity for a trait. Included a couple extra traits for good measure.

For example, https://github.com/zendframework/zf2/blob/master/library/Zend/Form/Fieldset.php#L297 could be:

class Fieldset extends Element implements FieldsetInterface
{
    use \Zend\Stdlib\Guard\ArrayOrTraversableGuardTrait;

    // ...

    public function setMessages($messages)
    {
        // php 5.4
        $this->guardForArrayOrTraversable(
            $messages, 'Messages', 'Zend\Form\Exception\InvalidArgumentException'
        );

        // or php 5.3
        \Zend\Stdlib\Guard\GuardUtils::guardForArrayOrTraversable(
            $messages, 'Messages', 'Zend\Form\Exception\InvalidArgumentException'
        );

        foreach ($messages as $elementName => $elementMessages) {
            $this->get($elementName)->setMessages($elementMessages);
        }

        return $this;
    }

    // ...

}

Please see http://verraes.net/2013/09/extract-till-you-drop/
Note: resubmitting to clean up failed rebase.

@texdc texdc Add Guard Traits
The GuardUtils can be used now.  The traits are for php >= 5.4 only.
b393e2f
@macnibblet
Contributor

Zend framework has a minimum requirement of PHP 5.3, introducing traits in core components will cause a BC which is not accepted until zf3

@ClemensSahs
Contributor

He write this for 5.3 too. the \Zend\Stdlib\Guard\GuardUtils is for php5.3 implementation.

👍

@ClemensSahs
Contributor

@texdc
I currently implement some more guard method's and I think we need perhaps a custom message as argument.
example case:

if (!extension_loaded('gd')) {
   throw new Exception\ExtensionNotLoadedException(
      'GD extension is required to use numeric font'
   );
}

here we want say why we required this Extension. But if we use custom messages will we want format the message with sprintf, too? If yes we need a "position specifier" for each guardMethode.

like:


    /**
     * Verifies that the data is an array or Traversable
     *
     * $message has tow placeholder:
     * - %1$s give the $dataName
     * - %2$s give the class or the type of $data
     *
     * @param  mixed  $data           the data to verify
     * @param  string $dataName       the data name
     * @param  string $exceptionClass FQCN for the exception
     * @param  string $message      Message format string
     * @throws \Exception
     */
    protected function guardForArrayOrTraversable(
        $data,
        $dataName = 'Argument',
        $exceptionClass = 'Zend\Stdlib\Exception\InvalidArgumentException',
        $message = '%1$s must be an array or Traversable, [%2$s] given'
    ) {
        if (!is_array($data) && !($data instanceof \Traversable)) {
            $message = sprintf(
                $message,
                $dataName,
                is_object($data) ? get_class($data) : gettype($data)
            );
            throw new $exceptionClass($message);
        }
    }
@texdc
Contributor
texdc commented Oct 30, 2013

I agree. This is a first pass to get approval and it can be enhanced.

On Oct 30, 2013, at 7:08 AM, ClemensSahs notifications@github.com wrote:

@texdc
I currently implement some more guard method's and I think we need perhaps a custom message as argument.
example case:

if (!extension_loaded('gd')) {
throw new Exception\ExtensionNotLoadedException(
'GD extension is required to use numeric font'
);
}
here we want say why we required this Extension. But if we use custom messages will we want format the message with sprintf, too? If yes we need a "position specifier" for each guardMethode.

like:

/**
 * Verifies that the data is an array or Traversable
 *
 * $message has tow placeholder:
 * - %1$s give the $dataName
 * - %2$s give the class or the type of $data
 *
 * @param  mixed  $data           the data to verify
 * @param  string $dataName       the data name
 * @param  string $exceptionClass FQCN for the exception
 * @param  string $message      Message format string
 * @throws \Exception
 */
protected function guardForArrayOrTraversable(
    $data,
    $dataName = 'Argument',
    $exceptionClass = 'Zend\Stdlib\Exception\InvalidArgumentException',
    $message = '%1$s must be an array or Traversable, [%2$s] given'
) {
    if (!is_array($data) && !($data instanceof \Traversable)) {
        $message = sprintf(
            $message,
            $dataName,
            is_object($data) ? get_class($data) : gettype($data)
        );
        throw new $exceptionClass($message);
    }
}


Reply to this email directly or view it on GitHub.

@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Stdlib/Guard/NullGuardTrait.php
+{
+
+ /**
+ * Verify that the data is not null
+ *
+ * @param mixed $data the data to verify
+ * @param string $dataName the data name
+ * @param string $exceptionClass FQCN for the exception
+ * @throws \Exception
+ */
+ protected function guardAgainstNull(
+ $data,
+ $dataName = 'Argument',
+ $exceptionClass = 'Zend\Stdlib\Exception\InvalidArgumentException'
+ ) {
+ if (is_null($data)) {
@weierophinney
weierophinney Jan 3, 2014 Member

use null === $data here.

@weierophinney weierophinney commented on the diff Jan 3, 2014
...ry/Zend/Stdlib/Guard/ArrayOrTraversableGuardTrait.php
+{
+
+ /**
+ * Verifies that the data is an array or Traversable
+ *
+ * @param mixed $data the data to verify
+ * @param string $dataName the data name
+ * @param string $exceptionClass FQCN for the exception
+ * @throws \Exception
+ */
+ protected function guardForArrayOrTraversable(
+ $data,
+ $dataName = 'Argument',
+ $exceptionClass = 'Zend\Stdlib\Exception\InvalidArgumentException'
+ ) {
+ if (!is_array($data) && !($data instanceof \Traversable)) {
@weierophinney
weierophinney Jan 3, 2014 Member

Import Traversable so it does not need to be globally qualified here.

@weierophinney weierophinney commented on the diff Jan 3, 2014
library/Zend/Stdlib/Guard/GuardUtils.php
+
+
+ /**
+ * Verifies that the data is an array or Traversable
+ *
+ * @param mixed $data the data to verify
+ * @param string $dataName the data name
+ * @param string $exceptionClass FQCN for the exception
+ * @throws \Exception
+ */
+ public static function guardForArrayOrTraversable(
+ $data,
+ $dataName = 'Argument',
+ $exceptionClass = self::DEFAULT_EXCEPTION_CLASS
+ ) {
+ if (!is_array($data) && !($data instanceof \Traversable)) {
@weierophinney
weierophinney Jan 3, 2014 Member

Import Traversable

@weierophinney
Member

Overall, I like this. I'm going to go ahead and merge, with changes as noted. @texdc -- would you be willing to start coming through individual components and submitting PRs that incorporate this? I like the idea of reducing boilerplate code. If you do, do a PR per component, as that will be easier to review.

@weierophinney weierophinney merged commit b393e2f into zendframework:develop Jan 3, 2014
@weierophinney weierophinney was assigned Jan 3, 2014
@texdc
Contributor
texdc commented Jan 3, 2014

Thanks! Yes, that's the idea. Do you have any components that should be prioritized?

On Jan 3, 2014, at 3:47 PM, weierophinney notifications@github.com wrote:

Overall, I like this. I'm going to go ahead and merge, with changes as noted. @texdc -- would you be willing to start coming through individual components and submitting PRs that incorporate this? I like the idea of reducing boilerplate code. If you do, do a PR per component, as that will be easier to review.


Reply to this email directly or view it on GitHub.

@arekkas
Contributor
arekkas commented Jun 26, 2014

Would guards against int, string, numeric make sense?

@weierophinney weierophinney added a commit to zendframework/zend-stdlib that referenced this pull request May 15, 2015
@weierophinney weierophinney Merge pull request zendframework/zendframework#5365 from texdc/Add_Gu…
…ard_Traits

[stdlib] Add guard utils and traits
2b7a3ad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment