Issues

ZF-209: Function names and use of TRUE and FALSE

Description

I'm slightly confused by function names given meaning numbers. It seems sometimes 'digit' is used and other times 'number'. Also they are shortened.

Functions: isDigits() isAlnum() getDigits() getAlnum() Should be isNumeric() isAlphanumeric() getNumbers() getAlphanumerics()

Also TRUE and FALSE seems to have been used routinely over true and false, personally I think the former is misleading as it implies they are constants and not language keywords. From what I can see the Zend Coding Standard doesn't specify which is acceptable.

Comments

general cleanup.

Great, seeing as someone has been assigned to this I should probably point out some other issues:


    public static function isBetween($value, $min, $max, $inc = true)
    {
        if ($value > $min && 
            $value < $max) {
            return true;
        }

        if ($value >= $min &&
            $value <= $max &&
            $inc) {
                return true;
        }
        
        return false;
    }

    public static function isBetween($value, $min, $max, $inclusive = true)
    {
        if ($value > $min && $value < $max) {
            return true;
        }

        if ($inclusive) {
            if ($value >= $min && $value <= $max) {
                return true;
            }
        }
        
        return false;
    }

    public static function isGreaterThan($value, $min)
    {
        return ($value > $min);
    }

    public static function isGreaterThan($value, $min)
    {
        return $value > $min;
    }

    public static function isInt($value)
    {
        $locale = localeconv();

        $value = str_replace($locale['decimal_point'], '.', $value);
        $value = str_replace($locale['thousands_sep'], '', $value);

        return (strval(intval($value)) == $value);
    }

    public static function isInt($value)
    {
        $locale = localeconv();

        $value = str_replace($locale['decimal_point'], '.', $value);
        $value = str_replace($locale['thousands_sep'], '', $value);
        //parentesis around cast necessary here? not sure
        return ((string)(int)$value) == $value; 
    }

    public static function isName($value)
    {
        return (bool) !preg_match('/[^[:alpha:]\ \-\']/', $value);
    }

What is this really checking because it sure as hell ain't checking 
if its somebody's name, that would be near impossible?

    public static function isPhone($value, $country = 'US')
    {
        if (!ctype_digit($value)) {
            return FALSE;
        }

        switch ($country)
        {
            case 'US':
                if (strlen($value) != 10) {
                    return FALSE;
                }

                $areaCode = substr($value, 0, 3);

                $areaCodes = array(201, 202, 203, 204, 205, 206, 207, 208,
                                   209, 210, 212, 213, 214, 215, 216, 217,
                                   218, 219, 224, 225, 226, 228, 229, 231,
                                   234, 239, 240, 242, 246, 248, 250, 251,
                                   252, 253, 254, 256, 260, 262, 264, 267,
                                   268, 269, 270, 276, 281, 284, 289, 301,
                                   302, 303, 304, 305, 306, 307, 308, 309,
                                   310, 312, 313, 314, 315, 316, 317, 318,
....

Are you planning on supporting every country? Here, I'd hope not.
You should probably rename to IsUsaPhone() and implement select countries only, 
i.e the ones with a phone naming system that is simple enough to get right.

That's it!

Hi Jayson

If you want I can take care of this whilst I am finishing some other fixes in Zend_Filter.

Please advise

Reassigning from Jayson to Bill Karwin, pending further triage.

Assigning to Darby.

Changing fix version to unknown.

Changing fix version to 0.9.0.

I disagree with the suggested changes to the naming convention. Digits is a very specific way to describe what's happening. The reason for Alnum is to remain consistent with the naming of the character type functions, which developers should already be familiar with.

I agree about the isName() comment. I'll remove that method, barring any objections.