LocaleBundle icon indicating copy to clipboard operation
LocaleBundle copied to clipboard

[Insight] Boolean should be compared strictly - in Event/FilterLocaleSwitchEvent.php, line 39

Open lunetics opened this issue 10 years ago • 2 comments

in Event/FilterLocaleSwitchEvent.php, line 39

With booleans, only strict comparison (with === operator) should be used to lower bug risks and to improve performances.

     *
     * @throws \InvalidArgumentException
     */
    public function __construct(Request $request, $locale)
    {
        if (!is_string($locale) || null == $locale || '' == $locale) {
            throw new \InvalidArgumentException(sprintf('Wrong type, expected \'string\' got \'%s\'', $locale));
        }

        $this->request = $request;
        $this->locale = $locale;

Posted from SensioLabsInsight

lunetics avatar May 09 '15 14:05 lunetics

I think we should only use if(!is_string($locale)) {. Edit: Or if(!is_string($locale) || !empty($locale)) { if empty strings should be forbidden.

is_string(false) = bool(false)
is_string(true) = bool(false)
is_string(NULL) = bool(false)
is_string('abc') = bool(true)
is_string('23') = bool(true)
is_string(23) = bool(false)
is_string('23.5') = bool(true)
is_string(23.5) = bool(false)
is_string('') = bool(true)
is_string(' ') = bool(true)
is_string('0') = bool(true)
is_string(0) = bool(false)

http://php.net/manual/de/function.is-string.php

ghost avatar May 18 '15 16:05 ghost

As explained here: https://github.com/lunetics/LocaleBundle/pull/161#discussion_r30524942 we should do if(!is_string($locale) || '' === $locale) {.

ghost avatar May 18 '15 16:05 ghost