Test if Something is an Integer

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Test if Something is an Integer

Post by Ollie Saunders »

So I was going to write a validation to test if something was an integer and I ended up writing this massive thing.
I'm curious, is there any way this could be improved?

Code: Select all

<?php
require_once 'Tilling/Validation/Exception.php';
/**
 * Validation to test if a string is intergal
 */
class Tilling_Validation_Test_Intergal
{
    /**
     * @var int|string
     */
    private $_maxInt = PHP_INT_MAX;
    /**
     * Test whether a value is an integer
     *
     * @param array $testSubjects strings to test
     * @param bool $signed set to false if only unsigned is allowed
     * @param int|string $maxInt the maximum integer size, if you do not have BC math installed this cannot be larger than PHP_INT_MAX
     * @return bool
     */
    public function invoke(array $testSubjects, $signed = false, $maxInt = PHP_INT_MAX)
    {
    	$this->_maxInt = $maxInt;
        $call = $signed ? '_signed' : '_unsigned';
        foreach ($testSubjects as $testSubject) {
            if (!$this->$call((string)$testSubject)) {
                return false;
            }
    	}
    	return true;
    }
    /**
     * Test if a signed integer string is intergal
     *
     * @uses _unsigned()
     * @param string $testSubject
     * @return bool
     */
    private function _signed($testSubject)
    {
        if ($this->_unsigned($testSubject)) {
            return true;
        }
        // Multibyte safe: Testing for single byte character
        if ($testSubject[0] == '-' && $this->_unsigned(substr($testSubject, 1))) {
            return true;
        }
        return false;
    }
    /**
     * Test if a unsigned integer string is intergal
     *
     * @throws Tilling_Validation_Exception if bcmath is required but no available
     * @param string $testSubject
     * @return bool
     */
    private function _unsigned($testSubject)
    {
        if (!ctype_digit($testSubject)) {
           // its obviously false if it contains non-digit characters
           return false;
        }
        if (is_int($this->_maxInt) && $this->_maxInt < PHP_INT_MAX) {
            // normal comparison is fine
            return $testSubject <= $this->_maxInt;
        }
        if ($this->_maxInt == PHP_INT_MAX) { // different rules for checking overflow
            // if its got the value of PHP_INT_MAX as a string then it's fine
            if ((string)$this->_maxInt === (string)$testSubject) {
                return true;
            }
            // if when cast to an integer it becomes PHP_INT_MAX this is an
            // indication that it was formerly out of range
            if ((int)$testSubject === $this->_maxInt) {
                return false;
            }
            return true;
        }
        // $this->_maxInt is greater than PHP natively handles
        // so we have to use bcmath
        if (!extension_loaded('bcmath')) {
            $errStr = 'Cannot compare number greater than PHP_INT_MAX without bcmath loaded';
            throw new Tilling_Validation_Exception($errStr);
        }
        return bccomp($testSubject, $this->_maxInt, 0) < 1; // 0 or -1 are valid
    }
}
and the unit test

Code: Select all

<?php
require_once 'Tilling/Validation/Test/Intergal.php';
class Tilling_Validation_Test_Intergal_Test extends Tilling_Test_Case
{
    /**
     * @var Tilling_Validation_Test_Intergal
     */
    public $inst;
    public function setUp()
    {
    	$this->inst = new Tilling_Validation_Test_Intergal();
    }
    public function testDigitsOnly()
    {
        $this->assertTrue($this->inst->invoke(array('1', '5', '94254334534'), false));
        $this->assertFalse($this->inst->invoke(array('1', '5', '9a'), false));
        $this->assertTrue($this->inst->invoke(array('1', '5', '94254334534'), true));
        $this->assertFalse($this->inst->invoke(array('1', '5', '9a'), true));
    }
    public function testSignChar()
    {
        $this->assertFalse($this->inst->invoke(array('-1'), false));
        $this->assertFalse($this->inst->invoke(array('1-'), false));
        $this->assertTrue($this->inst->invoke(array('-1'), true));
        $this->assertFalse($this->inst->invoke(array('1-'), true));
    }
    public function testWithinStorableRange()
    {
        $this->assertFalse($this->inst->invoke(array('99999999999999999999999')));
        $this->assertFalse($this->inst->invoke(array('-99999999999999999999999'), true));
        $this->assertTrue($this->inst->invoke(array(PHP_INT_MAX)));
        $this->assertTrue($this->inst->invoke(array(-PHP_INT_MAX), true));
    }
    public function testCustomMaxLessThanPHP_INT_MAX()
    {
    	$this->assertFalse($this->inst->invoke(array(500), false, 499));
    	$this->assertFalse($this->inst->invoke(array(-1), false, 499));
    	$this->assertTrue($this->inst->invoke(array(499), false, 499));
    	$this->assertFalse($this->inst->invoke(array(-499), false, 499));

    	$this->assertTrue($this->inst->invoke(array(-499), true, 499));
    	$this->assertFalse($this->inst->invoke(array(-500), true, 499));
    	$this->assertFalse($this->inst->invoke(array(500), true, 499));
    }
    public function testCustomMaxMoreThanPHP_INT_MAX()
    {
    	if (extension_loaded('bcmath')) {
    	    $this->assertTrue($this->inst->invoke(array('9999999999999'), false, $max = '9999999999999'));
    	    $this->assertTrue($this->inst->invoke(array('9999999999998'), false, $max));
    	    $this->assertFalse($this->inst->invoke(array('10000000000000'), false, $max));
    	} else {
    	    $this->expectException();
    	    $this->inst->invoke(array(1), false, '999999999999999');
    	}
    }
}
Tests were written first.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Code: Select all

if (strval(intval($var)) == strval($var)) 
{
    // it's a string with "Integer" value
}
:?
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

What if
  • You want to test for values larger or different to PHP_INT_MAX
  • The $var contains leading zeros
  • You only want to allow unsigned numbers
User avatar
Oren
DevNet Resident
Posts: 1640
Joined: Fri Apr 07, 2006 5:13 am
Location: Israel

Post by Oren »

I didn't bother to read the code... I'm with Jenk - this is too overkill :?

P.S See: http://us2.php.net/manual/en/ref.bc.php
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

ole wrote:What if
  • You want to test for values larger or different to PHP_INT_MAX

Code: Select all

preg_match('/^[0-9]$/', $string)
ole wrote:[*]The $var contains leading zeros

Code: Select all

preg_match('/^[0-9]$/', $string)
ole wrote:[*]You only want to allow unsigned numbers[/list]

Code: Select all

preg_match('/^[0-9]$/', $string)
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Yeah, this feels like massive overkill.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

:(
User avatar
JayBird
Admin
Posts: 4524
Joined: Wed Aug 13, 2003 7:02 am
Location: York, UK
Contact:

Post by JayBird »

Yeah, i thought i was missing something at first, but it does seem a little OTT
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

OTT and as complete as could possibly be,
That's how I like it you see,
Why don't you agree with me! :P

OK, you guys are probably right. So how would you suggest doing it?
Last edited by Ollie Saunders on Mon Apr 23, 2007 10:37 am, edited 1 time in total.
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post by John Cartwright »

Ahem.. :wink:
[url=http://forums.devnetwork.net/viewtopic.php?t=30037]Forum Rules[/url] Section 1.1 wrote:11. Please use proper, complete spelling when posting in the forums. AOL Speak, leet speak and other abbreviated wording can confuse those that are trying to help you (or those that you are trying to help). Please keep in mind that there are many people from many countries that use our forums to read, post and learn. They do not always speak English as well as some of us, nor do they know these aberrant abbreviations. Therefore, use as few abbreviations as possible, especially when using such simple words.

Some examples of what not to do are ne1, any1 (anyone); u (you); ur (your or you're); 2 (to too); prolly (probably); afaik (as far as I know); etc.
User avatar
JayBird
Admin
Posts: 4524
Joined: Wed Aug 13, 2003 7:02 am
Location: York, UK
Contact:

Post by JayBird »

I didn't see a rule break. "use as few abbreviations as possible"...i only used one, and it wasn't even really AOL speak! :P :P

The rule is kind of contradictory anyway by starting off saying use complete words, then ending saying use as few abbreviations as possible.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Yeah I would never have been able to write my limerick if OTT wasn't used.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

JayBird wrote:I didn't see a rule break. "use as few abbreviations as possible"...i only used one, and it wasn't even really AOL speak! :P :P

The rule is kind of contradictory anyway by starting off saying use complete words, then ending saying use as few abbreviations as possible.
Not to mention the irony of using "AOL." :wink:
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post by Ambush Commander »

First thing you need to do is fix your naming. Integral not intergal.

Next, we need to know what exactly you're trying to accomplish with this class. From my glance over, you're trying to do a lot more than just test whether a number is an integer or not, and you've gone through considerable gymnastics to do that. What are your objectives?
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Thank you, Ambush Commander, for actually asking a question as opposed to outright bashing. Perhaps the others can learn from you.
What are your objectives?
Bullet-proof validation of integers from strings. I wrote this with databases, and their many different sizes and types of integer, in mind. With this validation called correctly you can be sure that you are getting an integer and that it will fit in your database.
Post Reply