- 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
Clearly he was trying to order by relevance to the search term.
He did an awful job of it, but that's what he was trying to do
Admin
The code is pretty clear to me. The only problem could be a missing explain text in the UI for the sorting logic.
It sorts by:
It is even indented and commented... The WTF here is just the article itself.
Admin
TRWTF is leaving in TODO comments professing you don't know what you deleted. What are you going TODO about it and when?
Admin
Yeah this is definitely a search by relevance, we use a similar script for searching. The ordering is terrible though.
Admin
The ordering is probably what was originally asked for (show the most relevant matches first.... or something equally vague). The real WTF here is any kind of experienced programmer who can't work out what that code is attempting to do....
Admin
Ahh, memories from 23 years ago... Custom "Relevance" sorting. This does not even begin to touch on how complex a solution can get (and how much is buried inside of search engines such as Google).....
Agree that the WTF is with the people who could not understand the code. Heck a few simple test cases (against a dummy DB or even table) would have illustrated it perfectly.
Admin
Since Caleb just began an internship , the "more experienced programmer" could just be a recently graduated programmer with 1 month of experience...
Though TRRWTF is the TODO that will never be done because they just don't know what to do in the frist place.
Admin
If they are complaining about that query they should consider themselves lucky. I've written my share of complicated SQL, often going to unnecessary (in hindsight) lengths to cover edge cases that never materialized. I do try to explain the weirder portions of my queries though and this probably would have qualified as weird.
Admin
Nice that they left the SQL injection vulnerability.
Admin
Does the key word involve Johnny Tables?
Admin
Why are you mentioning my brother?
Admin
Wait! Was his name Caleb or Cabel? Or does Caleb suffer from multiple-personality disorder?
Admin
Oh, if only this really were unique... why do so many otherwise sensible developers decide that they can throw away 50 years of software design principles when they are working on a web site? Not that I suspect that this moron was in any way sensible, but seeing others do this tends to encourage the lazy and incompetent, like we see here.
As for the sorting order, well, it's... ugh. I agree that there seems to have been some method to the madness, but without any corroborating documentation, how the fuck can you tell? More to the point, when you do a relevance-ordered search, it is usually a good idea to chunk the relevance groups on the page itself in a way that makes it clear that it is a relevance-ordered layout. Even Google - who are actually quite slick about not needing such visible grouping - show a ranking of the relevance with the links. C'mon, people!
And then you replace this snerk well-tuned relevance ordering with... ORDER BY on a single value, while admitting that you're too stupid to understand what's going on. Brillant!
Addendum 2016-04-28 10:48: Eh, I got confused about Google, forget I said such a stupid thing.
Admin
Neither. It was Cable from the X-Men, under two different disguises Either that, or TDWTF's editors are not the most thorough people in the world whenever it comes down to avoiding typos.
Admin
It's rudimentary, but outside of the injection it's really not even that terrible. There are maybe more elegant ways to do it, and even if I were to do it this way I'd probably leave off the "ends with" checks (I don't see why that adds importance, though who's to say this wasn't a requirement), but there are worse TDWTFs.
More importantly, noob or not, I'm not sure how you don't see what it's doing there. They would even be, more or less, sorted by name.
Admin
The ordering is attempting to put the most relevant results first - they should have been able to figure that out by looking up what % and $ mean in a LIKE condition, but its not a sensible query, because of the inefficiencies of how it's done. So it was an intern, if you've completed an IT degree you should be able to handle some basic logic in a SQL script.
What you'd expect is some kind of ordering option that allows you to sort matches by Relevance (using something like this, although there's much better ways to do this kind of scoring - like tokenisation and bag of words models), Name, Date, Price, etc. Then you can use this order when it's useful, but allow selecting a 'human comprehensible' order if you prefer.
It's probably more unfortunate that careless programmers rip out code without bothering to understand it first, although if you're going to come up with a complex sort option understanding the performance implications as well as its comprehensibility are handy, too.
Admin
The reason that the ordering isn't consistently applied is because the ORDER BY clause appears inside a subquery. This actually isn't valid SQL at all, I think -- it should throw an error when you try to execute it -- but even if it syntactically valid, ordering is only guaranteed to work when applied to the outermost query.
The reason that it /sometimes/ works is because the query plan used to execute this query is built at run-time -- sometimes (likely when the inner query returns a small set of results), a simple execution plan is chosen for the outer query and the sorted data in the inner query returns the ordering of the output. However, in other cases, the optimizer first sorts the inner query according the key provided then /re-sorts/ the data by some other key to make the outer query execute in a more efficient way. When this occurs, obviously, the final output won't be sorted as expected.
Admin
Obviously the results from testing the query confused Caleb/Cabel, and they made the same mistake most of us made.
It also says to me the predecessor coded and didn't bother testing or they would have seen the unexpected results for that query. "I'll just copy and paste this block here, and then wrap it like this... cool, it compiles!"
Admin
Frist? No?
Admin
There are no classes. It is JavaScript, there are objects and prototypes.
Admin
I don’t know anything about SQL, but couldn’t the first three lines of:
exact term in name name starts with term name ends with term name contains term
be removed without changing what the routine does?
Admin
Pretty simple to parse code but if you're thinking you need relevance as a part of your search system then you should probably consider using a proper full-text search system like ElasticSearch to do it.
Even if you're not doing proper full-text search, it's still generally easier to adjust this kind of stuff in ES...
Admin
I'm thinking that what happened was more along the lines of it USED to order by relevance/order and by name, but some time later some other requirement involving media_id came down that resulted in the LEFT JOIN being bunged on the end. Nobody in the development team of one person thought through the consequences of not relocating the order by criteria to the outer query.
Admin
I am very confident that JavaScript is not mentioned in the article at all. And the code snippets are from PHP (which does have classes). So I don't know why you decided to mention that.
Admin
That is clearly a typo. Caleb is a member of the Cabal and so can be referred to in either context.
Admin
Hi, I am Caleb, the intern who submitted this story. Allow me to clarify a few things for everyone.
First, I have been programming for > 3 years and have individually published over 40 projects and contributed to numerous projects. I am no “mere intern.” I have done a ton of stuff in 3 years but not had a “real job” until now. I have never been described as lazy, careless, or incompetent”. In fact, I’ve been called a perfectionist to a high degree when it comes to programming. I do things right and proper, no incompetent, half-hearted work here. My college instructors had a hard time even teaching me because I was able to easily comprehend the work and be a week or two ahead of schedule (that is, if I had not already self-taught myself everything at least 6 months before out of self-initiative, way before I knew I would be taking that class).
I and my co-worker (who is a > 5yr programmer and was around when the system this query came from was hacked together) did our best to comprehend the query. In fact, we did finally understand how it worked. I took it home and worked it out. I even had a complete database to test it against. In the end, we concluded it was better to replace the query with something simpler. The query was boosting irrelevant results as well as filtering out and not displaying other, more relevant results. Additionally, it did not even respect certain conditions of search (displaying unpublished products). Rest assured, we found out exactly what it did, even with lack of documentation, but it made more sense to completely replace it.
The timeline is not correct. The other guy had been gone for a month already, and this story took place about a week into the job. The changes were not pushed into production for another two weeks, and when my super saw it side-by-side to the original query a few days before production push, he fully agreed the simpler version returned better results.
Per the query formatting, comment, and in-place SQL injection vulnerability, that was done only for this article. The query was all on a single line. I formatted it, left the vulnerability in for reading-comprehension (not to mention that is how it really was), and wrote the comment for further reader-clarity. Same goes for the vulnerability in the revised query. I deliberately submitted it that way while the actual system uses a prepared statement.
Finally, the TODO comment I wrote read along the lines of “The previous search query used a complicated relevance ordering but did not a good job. For now, we'll sort by name and later improve the relevance.” Why the heck would I leave a comment saying I had no idea what I replaced??? That’s like something being struck from a governing article and someone requesting a note be left saying something used to be here. No, you get an older copy and see what was removed. I would not do what was said I did. I have source control if I ever need to go back and look at it originally.
So yea. While you are all entitled to your opinions, and I do respect them, it never feels good to be labeled as an idiot and being called TRWTF when a lot of the relevant details are missing, details needed to form an accurate opinion.
Admin
If you are admitting that the code was simplified for editorial purposes, why do you blame other people for judgements based on the available information? Just post the real code, I think that most of the readers are experienced developers anyway.
Admin
Yeah, but it's crap though, isn't it? If it's not doing what's intended, it's wrong.
Admin
I don't even SQL, and I figured that much out.
Admin
Looks like sorting by relevance, as in results where name is exactly "keyword" will appear first, then names that start with "keyword", then names that contain "keyword", and so on. Too bad that each individual list is not sorted so it looks like a giant mess.