- Feature Articles
- CodeSOD
- Error'd
-
Forums
-
Other Articles
- Random Article
- Alex's Soapbox
- Announcements
- Best of…
- Best of Email
- Best of the Sidebar
- Bring Your Own Code
- Coded Smorgasbord
- Mandatory Fun Day
- Off Topic
- Representative Line
- News Roundup
- Editor's Soapbox
- Software on the Rocks
- Souvenir Potpourri
- Sponsor Post
- Tales from the Interview
- The Daily WTF: Live
- Virtudyne
Admin
Pretty good WTF! Nice to see the PHP crowd keeping up the standards
Admin
Hold on, if
strtotimereturns seconds as an integer, andstrcmpimplicit-tostringifies those integers, isn't there a chance that for some values, this comparison and the sort is broken?Admin
Yeah, I thought the same thing. It will only work if all values have the same number of decimal digits.
strtotimereturns a Unix timestamp so if there are values both before and after 2001-09-09 (10e9 seconds) the order will be wrong. Same for all powers of ten though 10e8 seconds is 1973-03-03, so that's even less like to be hit in practiceAdmin
That seems a little harsh. After all, you are already using PHP, so lower your standards. You've already committed yourself to the quick & dirty approach, and "a hook as a string variable" is actually useful.
Admin
Indeed, you could use a URL parameter as a function.
Admin
That's my thought as well, having looked at the documentation it doesn't clearly specify. It mentions "less than/greater than" without defining what that means in context. In PHP, "1234" would be a greater integer than "01234", but as strings I'd consider the latter the greater one...
Admin
Runs screaming from the room.
Admin
I once was a developer of a version of Pick BASIC. The language had a "RETURN TO" statement. You could "GOSUB" to a subroutine but "RETURN" to someplace else in your code. Much worse than calling a function passed as a string. This is available in other languages like BASH, PERL, JavaScript.
Admin
This is why programming is hard. The code needs to be agnostic about the nature of the data being passed into it; so it should not assume it is already sorted. And it should not necessarily assume that the strings being passed in are inherently sortable. I actually think the developer did a fine job implementing this code, given those considerations. (Sure, there is some optimization that could be done, but the point is that there is no coupling between the database and the code).
The fact that from a 30,000 foot level, we know both the format of the strings and the source of the data (i.e, that it is already sorted) is due to the implicit coupling of the database and this method. And we should expect (because of TDWTF) that implicit coupling will break at some point, breaking the application.
It's better to sort an already-sorted list, than allow the code/application to break because some DBA decided to remove the sorting from the database query.
Admin
At the same time, certain operations should happen in the layer best suited to doing them. Sorting is a prime example- if you have a database, sort in the database, where the sort can be optimized with indexes or the table structure itself.
Also, use meaningful types- PHP has a DateTime type. Using a string (and a Unix timestamp) is the wrong way to solve this problem. I'd go so far as to argue that using a string to represent a date is always a mistake (yes, even when you're using ISO8601), and you should always favor some sort of date structure or object- because as you point out, making the contract of a function dependent on date strings having the correct format is itself risky.
Admin
Bit early for April Fools, isn't it? But in case you were serious:
Yes, you should assume what you know about your data. In a typed language, the dates would have an appropriate date/time type, and therefore be inherently sortable. (Null may be a concern, but this code doesn't handle it either, so let's ignore it.)
If you can't guarantee it statically, you document the constraints rather than redoing the work.
A "fine job" indeed that doesn't even sort correctly for older dates (see other comments).
If a DBA randomly removes parts of queries, we very much expect things to break. Why should this be any different here?
Actually quite the contrary: Say in the future management decides to sort by whatever instead of date. DBA duly implements the change and it doesn't work. Why? Because some clever code knows better and messes up the correct sorting returned from the DB.
Admin
bash, really, how? Do you mean "trap"? If you, I'd say that's more akin to exception handlers in other languages (which can be abused, sure, but are not as weird as just returning to someplace else).
Admin
shudder This is PHP so I'm going to assume the worst case here
Everyone: Cross-site scripting is bad. PHP: Hold my beer...
Admin
Is this ability really that bad? And is it really that far off from dispatch tables that use strings as keys to call functions?
Admin
Being able to use function names as values is a relatively recent addition to PHP, for many years you had to use strings. And for much of its history, defining functions dynamically used
create_function(), which takes string arguments -- there were no lambdas. These have been obsoleted, but there's still plenty of the old style code around.Admin
Admin
Not all that long ago, someone on YouTube posted a video touting the wonders of the latest version of PHP, I made some snarky comments about older code. The creator responded along the lines of "You have something against backward compatibility?" I essentially said "if it lets PHP programmers keep writing the same old garbage, then I do."
Admin
They are probably using strcmp() because it conveniently returns -1, 0 or +1 to indicate how the arguments compare, which is exactly what usort() needs the callback function to return. From the documentation on usort():
"The comparison function must return an integer less than, equal to, or greater than zero if the first argument is considered to be respectively less than, equal to, or greater than the second."
Admin
Closures in PHP are 15 years old. 15! Many other silly PHPisms are heavily frowned upon these days. I've not actually used it since about then, but it looks (or can with a linter) much better than the hodgepodge JavaScript I'm stuck with these days, which can only change so much due to compatibility. Proper types with proper compile time checks, how I long for you.
Admin
Sure, since at this point they have integers and I couldn't think of any function that can be applied to integers and has this property - as hard as I try ...
Admin
This routine does not actually produce a correctly sorted list--it will exhibit the standard flaw of string comparisons of integers putting 10 before 2 and so on. It's possible this is a desired behavior--deliberately creating a "bug" so it behaves like it always has.
It reduces it to -1/0/1. I can easily see someone lazy using an existing, inefficient approach rather than writing a better one. I don't speak PHP but the Sign function comes to mind and I would think there's some equivalent.
Admin
PHP doesn't seem to have a sign function, but one can make use of the fact that TRUE is 1.
return ($a > $b) - ($b < $a)
But usort just requres them to be positive / zero / negative, not actually 1 / 0 / -1, so one can use just
return $a - $b
Admin
"The comparison function must return an integer less than, equal to, or greater than zero if the first argument is considered to be respectively less than, equal to, or greater than the second."
...which doesn't say that the integer returned has to be -1, 0, or 1.
Here's a challenge: come up with a function that takes two integers n and m, and returns an integer that is less than, equal to, or greater than 0 if n is respectively less than, equal to, or greater than m.
As for using strings as callable functions ....
usort($data, cmp(...))would do the same job without that.Admin
The fact that they are not using closures says this code is old, and likely predates the DateTime class