Oh, snap.
1 2 3 4 5 6 7 8 9 10 |
|
This little bundle of joy really caught my attention. It’s the exact type of code that drove me to launch CSI: PHP.
Context
Apparently, the developer wanted is_int(sqrt(16))
to return true. Since the return value of sqrt()
is always a float, the dev wrote is_really_int()
to return true whenever the return value of sqrt()
could be accurately represented as an integer.
The horror
Fair enough. I even feel dude’s pain. But the cure is worse, oh so much worse, than the disease. Here are a few reasons why.
- Userland method and function names should be descriptive. I want to be able to know exactly (or have a good idea, at least) what’s going to happen when I call your function. If
is_int
() returns false, then why is asking, “Really?” going to change matters? - Passing by reference? Why? Functions prefaced by “is” should do one thing only: test something and return a boolean. Changing the value of
$var
when the input can be an int violates this principle, is totally unexpected, and will lead to unexpected behavior in your application. - This is the kind of code that you’ll look at 6 months later and think, “Now what the hell does this do again?” You’ll want to shoot yourself in the face when you finally figure it out.
- This is the kind of code that turns a maintenance developer into a homicidal maniac, and the maintainer is a bad mother— (Shut your mouth!). I’m just talkin’ about funkatron! (Then we can dig it)
- The function returns wildly unexpected results (and I can prove it).
Something better?
Again, I feel dude’s pain. If I get a float that can be represented as an integer, I’d like to be able to test for that without jumping through hoops. I whipped this up to try and solve the problem without the issues caused by is_really_int()
.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 |
|
It’s true that I might wonder what the hell valueCanBeInt()
does 6 months down the road, but a quick glance at the documentation and the function will make it clear. I might even come up with a better name for the function at that point and do a little refactoring. I’d be able to refactor with confidence, as I’ve written some …
Unit tests
Like I said, this one really captured my attention. I’ve written some unit tests to demonstrate to myself and to you the dangerous side effects of is_really_int()
. I ran the same tests against valueCanBeInt
. The code and tests are available on github, if you’re interested.
UPDATE: I totally forgot to thank @Tyr43l for the submission. Thanks Ferenc!