Page 2 of 2
Posted: Tue Apr 24, 2007 3:34 am
by Jenk
You've got nothing to test if it is a "smallint", "bigint" or other variation.. as posted before;
does exactly what your class does, without the drawbacks your class has.. for a start, you are dependent on ctype - which is an optional extension.
As for the bashing, well this is the Critique forum, it's not the "Pat on the back and a congratulations" forum.

Posted: Tue Apr 24, 2007 3:46 am
by CoderGoblin
ole wrote:Yeah I would never have been able to write my limerick if OTT wasn't used.
Ah but you see here you do things too small... A limerick is five lines
And for those you don't know OTT is stands for Over The Top.
Posted: Tue Apr 24, 2007 3:49 am
by Ollie Saunders
You've got nothing to test if it is a "smallint", "bigint" or other variation
Yes I do
Code: Select all
public function invoke(array $testSubjects, $signed = false, $maxInt = PHP_INT_MAX)
for a start, you are dependent on ctype - which is an optional extension
It's part of the core, and is faster and more secure that PCRE. But if it makes you happy you could write this:
Code: Select all
private function _unsigned($testSubject)
{
// its obviously false if it contains non-digit characters
if (function_exists('ctype_digit')) {
if (!ctype_digit($testSubject)) {
return false;
}
} else if (!preg_match('/^\d+$/', $testSubject)) {
return false;
}
Your regex of /^[0-9]+$/ won't allow signed strings - and before you say neither will mine look at the way _unsigned() is called. Nor will a regex alone fail on 99999999999999999999999999999999 which is obviously too big to be stored anywhere. and for that matter what if PHP is compiled without PCRE? To be honest, checking for the existance of things that I know exist on my server is just adding bloat where it isn't necessary - where as I do use both signed, unsigned, big, small and other types of integers in my databases.
Incidentally, for everyone's information, Jenk does have something personal against me that explains his behaviour.
Posted: Tue Apr 24, 2007 6:34 am
by JayBird
ctype is only available by default on PHP installations => PHP 4.2.0.
I think it would help if you listed all the advantages of using this code snippet over the smaller snippets people have suggested that do the same thing...otherwise people will just presume your using way too much code to do something that can be done in 1 line.
Posted: Tue Apr 24, 2007 1:55 pm
by jmut
How about? Nobody mentioned this one.
Posted: Tue Apr 24, 2007 2:20 pm
by Chris Corbyn
jmut wrote:How about? Nobody mentioned this one.
Doesn't just check for integers.
Posted: Tue Apr 24, 2007 3:34 pm
by Ambush Commander
Aight. The killer feature in this case is making sure the integer doesn't overflow. This is slightly more complicated than one would expect, since counting the number of digits is not sufficient.
Before I start rambling, keep in mind that nearly all databases will gracefully handle insertions of numbers that overflow by clipping them, and if this is a necessary error condition MySQL can operate in Strict mode, which will fail the query if there's an overflow.
Now, in general, the size of the number we are validating is constrained by the number of bytes/bits we're allowed to use. PHP, internally, uses 32-bit integers (4 bytes), so for any numbers that are within this range +/-2147483648 we can use PHP's regular integer comparison functions to determine whether or not 888 will fit in a TINYINT column (it won't).
The main probably I see, as of right now, is that you expect the user to know what the max value is. They probably don't. All they know is that they're validating for a TINYINT: your class needs to account for that. Also, you need some unit-tests. I believe that -128 is a valid value for a signed TINYINT, but 128 is not.
Posted: Wed Apr 25, 2007 12:43 pm
by Maugrim_The_Reaper
ctype is only available by default on PHP installations => PHP 4.2.0.
Ctype is available in approximately 95% of all PHP installations vs 97% for PCRE. PCRE is also entirely optional (oh yes it is!) and was only available by default since PHP 4.2.0. Along with a lot of extensions oddly enough... If a PHP installation does not have ctype enabled than it needs updating...not pandering.
Posted: Wed Apr 25, 2007 4:43 pm
by Ollie Saunders
The killer feature in this case is making sure the integer doesn't overflow. This is slightly more complicated than one would expect, since counting the number of digits is not sufficient.
Are you implying my my code insufficient in this regard?
keep in mind that nearly all databases will gracefully handle insertions of numbers that overflow by clipping them, and if this is a necessary error condition MySQL can operate in Strict mode, which will fail the query if there's an overflow
For some reason I wanted to write the most complete integer validation and I just thought "hey if its too big its not going to be an integer" and wrote the validations. I think you guys have done a pretty good job of convincing me that it is not the responsibility of integer validation to ensure an integer is within storable range and if you want to do that there should be a separate numeric range validation (which I have). That keeps thing small, simple and single responsabilitified
we can use PHP's regular integer comparison functions to determine whether or not 888 will fit in a TINYINT column (it won't).
And bcmath for outside of that.. its in my code towards the bottom.
The main probably I see, as of right now, is that you expect the user to know what the max value is. They probably don't. All they know is that they're validating for a TINYINT: your class needs to account for that.
Yeah, that would make it more useful. But, going back to the main issue, it is necessary to do this at all? I always validate a string is less than 255 chars if its going in a varchar because I don't want the user to experience truncation unexpectedly. Perhaps the principle only applies to small integers where an overflow is more likely. And again you probably want a separate range validation because you don't want a "not an integer" error when you really me "number is too large".
Also, you need some unit-tests.
Besides the one I posted?
I believe that -128 is a valid value for a signed TINYINT, but 128 is not.
Yes, you are right.
Posted: Wed Apr 25, 2007 4:48 pm
by Ambush Commander
You do a very good job of explaining the objections. Go with your new idea: simplistic integer checking combined with numeric bounds checking.
Posted: Wed Apr 25, 2007 4:54 pm
by Ollie Saunders
* rubs eyes *
Is it me or did we just reach a unequivocal agreement and the devnet forums?! Maybe there is hope.
Thanks AC.
d11wtq | Noticed this was your 2007th post, in year 2007
Posted: Sun Apr 29, 2007 9:58 am
by RobertGonzalez
In the spirit of modular coding, shouldn't there be classes for:
- checking if a value is an integer
- checking if an integer is within bounds
- exceptions for both cases
That would keep singular responsibility tied to each class, and decouples the integer checks from the boundary checks. Or am I not reading this correctly?
Posted: Sun Apr 29, 2007 10:02 am
by Ollie Saunders
I am going to have separate classes for is integer and within range.
I am not going to throw an exception if a validation fails because I do not believe a failed validation is a particularly serious or unexpected failure.
Posted: Sun Apr 29, 2007 11:04 am
by feyd
Exceptions aren't solely for serious or unexpected errors. They are a facility for passing the error handling to a layer above the current one. That may be the calling layer, but could be higher.
Posted: Sun Apr 29, 2007 12:14 pm
by Ollie Saunders
Exceptions aren't solely for serious or unexpected errors.
But that is what they are most commonly assoicated with and this is important when you consider that a one of the good reasons for using them is the readability they add.
They are a facility for passing the error handling to a layer above the current one. That may be the calling layer, but could be higher.
And this is probably the only functional advantage to using them and inappropriate, given my current implementation. Presently I'm using a component paradigm to invoke validations and store the error message and its destination, if the destination is missing no error message is displayed, not optimal but better than fatality surely.