CodeSOD: The Chosen Solution
Before today's article, I'm gonna humblebrag a bit, minus the humble part. Peregrine launched last night, at 2AM, and by 3AM, we had communication with the spacecraft. I have done very little software on this particular mission, but code I touched is in space right now. My name is on a plaque on that lander. Also, the code I touched has no control over the mission, and is in no way, shape, or form in the control loop, so if anything goes wrong it's definitely not my fault.
Mac bears some responsibility for today's code, in that, he "fixed" it. That is to say, he was working on an unrelated bug, but this particular block of code was causing additional problems, so he did the bare minimum to make it stop bothering him so he could work on his actual ticket.
This JavaScript running on their page would allow the user to check a checkbox, navigate away from the page, and then if they hit "back" (something Mac was doing a lot while debugging), the checked checkbox would gradually migrate down the page for some weird set of JS and CSS reasons.
This revised code stops that behavior.
if (!$('body').hasClass('type-car') && !$('body').hasClass('page-lists-members') && !$('body').hasClass('page-tools') && !$('body').hasClass('page-findhelp') && !$('body').hasClass('page-active') && !$('body').hasClass('page-dash') && (!$.browser.msie && $.browser.version <= "8") && !$('body').hasClass('type-event') && !$('body').hasClass('page-add-event')) { //I didn't write this monstrosity, I fixed it. Look further back in the git logs if you like staring into hell... var example_of_really_bad_js_wrapped_in_really_bad_js = $('select').not('#edit-field-car-ref-und, .views-widget-filter-date_filter select'); example_of_really_bad_js_wrapped_in_really_bad_js.chosen(); example_of_really_bad_js_wrapped_in_really_bad_js.change(function(){ if($('.chzn-results li').length < 10) { $(this).each(function(){ $('.chzn-search').hide(); }); } }); }
I love that one-line if condition, but honestly, that's small change compared to what's in the condition- why so many checks against body-level classes? Certainly a code stench, that they couldn't figure out a better way to decide when to execute this functionality.
In comparison, the body of the condition is a pretty minor level of ugly. Indentation as it was in the submission.
This is the kind of bad code that you can just sit back and look at, and not have to think too hard about. Even "fixed", it's just ugly and bad. We can all be glad this isn't in our codebase, and just move on with life.
[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!