CodeSOD: Ancestry Dot Dumb

Damiano‘s company had more work than staff, and opted to hire a subcontractor. When hiring on a subcontractor, you could look for all sorts of things. Does their portfolio contain work similar to what you’re asking them to do? What’s the average experience of their team? What are the agreed upon code quality standards for the contract?

You could do that, or you could hire the cheapest company.

Guess which one Damiano’s company did? If you’re not sure, look at this code:

if(jQuery('table').hasClass('views-view-grid')){
  var EPid= ".views-view-grid";
  jQuery(EPid +' td').each(function(){

   if(!jQuery(this).parent().parent().parent().parent().parent().hasClass('view-article-in-right-sidebar') && !jQuery(this).parent().parent().parent().parent().parent().hasClass('view-offers-in-right-sidebar')){
    var title = jQuery(this).find("h2 a").html();

    var body = jQuery(this).find(".field-name-body").html();
    var datetime = jQuery(this).find(".field-name-field-event-date-time").html();
    var flyer = jQuery(this).find(".field-name-field-flyer a").attr("href");
    var imageThumb = jQuery(this).find(".field-name-field-image-thumb").html();
    var readMore = '<a href="'+jQuery(this).find("h2 a").attr("href")+'" class="read-more">READ MORE</a>';

    var str = '<div class="thumb-listing listpage">';

    if(title != null && title != ""){
      if(imageThumb && imageThumb != "" && imageThumb != null)
        str = str + imageThumb;
      if(datetime && datetime != "" && datetime != null)
        str = str + '<div class="lp-date ">'+datetime+'</div>';
      str = str + '<div class="lp-inner clear"><div class="lp-title">'+title+'</div>';
      str = str + body + '</div><div class="sep2"></div>';
      str = str + readMore;
    }
    if(flyer)
      str = str + '<a class="download-flyer" href="'+flyer+'"><?php if(isset($node) && $node->type == "events"){ echo 'download the flyer'; }else {echo 'download the article';} ?></a>';

    str = str + '</div>';
    jQuery(this).children('.node').remove();

    jQuery(this).append(str);
  }
});

This was in a Drupal project. The developer appointed by the contractor didn’t know Drupal at all, and opted to build all the new functionality by dropping big blobs of JavaScript code on top of it.

There’s so much to hate about this. We can start with the parent().parent() chains. Who doesn’t love to make sure that your JavaScript code is extremely fragile against changes in the DOM, while at the same time making it hard to read or understand.

I like that we create the EPid variable to avoid having a magic string inside our DOM query, only to still need to append a magic string to it. It hints at some programming by copy/paste.

Then there’s the pile of HTML-by-string-concatenation, which is always fun.

But this couldn’t be complete without this moment: <?php if(isset($node) && $node->type == "events"){ echo 'download the flyer'; }else {echo 'download the article';} ?>

Oh yeah, buried in this unreadable blob of JavaScript there’s a little bonus PHP, just to make it a little spicier.

The entire project came back from the contractor in an unusable state. The amount of re-work just to get it vaguely functional quickly outweighed any potential cost savings. And even after that work went it, it remained a buggy, unmaintainable mess.

Did management learn their lesson? Absolutely not- they bragged about how cheaply they got the work done at every opportunity, and entered into a partnership agreement with the subcontractor.

Source: Read More