rename "setLocationOnEvent" to "setLocationOnValChange"

gistfile1.js

/*****************************************************************************
 *   __  __            _       _             ___              _
 *   \ \/ _\     /\/\ (_)_ __ | |_ _   _    / __\ __ ___  ___| |__
 *    \ \ \     /    \| | '_ \| __| | | |  / _\| '__/ _ \/ __| '_ \
 * /\_/ /\ \   / /\/\ \ | | | | |_| |_| | / /  | | |  __/\__ \ | | |
 * \___/\__/   \/    \/_|_| |_|\__|\__, | \/   |_|  \___||___/_| |_|
 *                                 |___/
 *
 * Identifying and Eliminating Code Smells
 *
 * Originally by Rebecca Murphey
 * Senior Software Engineer at Bazaarvoice
 * @rmurphey • rmurphey.com
 *
 * Forked by Miller Medeiros
 * @millermedeiros • blog.millermedeiros.com
 *
 *****************************************************************************/
















// THE SMELL

function updateDates () {
  var startDate = $("#start-date").datepicker("getDate");

  $("#menu-item-1 .day").text( startDate.getDate() );
  $("#menu-item-1 .month").text( Util.monthToText(startDate.getMonth() ) );
  $("#menu-item-1 .year").text( startDate.getFullYear() );

  var endDate = $("#end-date").datepicker("getDate");

  $("#menu-item-2 .day").text( endDate.getDate() );
  $("#menu-item-2 .month").text( Util.monthToText( endDate.getMonth() ) );
  $("#menu-item-2 .year").text( endDate.getFullYear() );
}


















// THE FIX: functions

// in fact you are not "updating the dates" you are just displaying the current
// values, so "updateDate" is not a good name for it
function renderStartAndEndDates () {
  displayDate( '#menu-item-1', $('#start-date').datepicker('getDate') );
  displayDate( '#menu-item-2', $('#end-date').datepicker('getDate') );
}

// better to pass the Date as an argument so the logic is decoupled from the
// datepicker implementation (supose we need to change the behavior to display
// arbitrary date objects)
// also there is no point in keeping this function inside the 
function displayDate (target, date) {
  // wrap target into a jQuery collection, so you can pass selectors, elements
  // and/or jQuery collections
  target = $(target);

  // no need to query for same element multiple times, better to use `find`
  // avoid duplication, better performance and more organized
  target.find('.day').text( date.getDate() );
  target.find('.month').text( Util.monthToText( date.getMonth() ) );
  target.find('.year').text( date.getFullYear() );
}









// OPPORTUNITY: associate date picker via data- attr and use a class name to
// identify which elements should use this logic
//
// <fieldset id="menu-item-1" class="js-datepicker-date" data-datepicker="#start-date">
//   <span class="day"></span>
//   <span class="month"></span>
//   <span class="year"></span>
// </fieldset>

function updateDates () {
  $('.js-datepicker-date').each(function(i, target){
    var datepicker = $( target.attr('data-datepicker') );
    var date = datepicker.datepicker('getDate');
    displayDate(target, date);
  });
}

function displayDate (target, date) {
  target = $(target);
  target.find('.day').text( date.getDate() );
  target.find('.month').text( Util.monthToText( date.getMonth() ) );
  target.find('.year').text( date.getFullYear() );
}














// THE SMELL

$('#countyLocation').change(selectedLocation);

function selectedLocation() {
  var selected = $('#countyLocation').val();

  if (selected == "Ireland") {
    var Ireland = new google.maps.LatLng(53.527248, -8.327637);
    map.setCenter(Ireland);
    map.setZoom(6);
  }

  if (selected == "Clare") {
    var Clare = new google.maps.LatLng(52.988337, -9.102173);
    map.setCenter(Clare);
    map.setZoom(8);
  }

  if (selected == "Dublin") {
    var Dublin = new google.maps.LatLng(53.399707, -6.262207);
    map.setCenter(Dublin);
    map.setZoom(9);
  }
}

















// THE FIX: objects

// we store the data as a "constant" at the top of file (or separate module)
// since this is a "configuration" and will probably need to be updated
// multiple times. It is not a good idea to have data scattered in the middle
// of the code
var LOCATIONS = {
  "Ireland" : {
    lat : 53.527248,
    lng : -8.327637,
    zoom : 6
  },
  "Clare" : {
    lat : 52.988337,
    lng : -9.102173,
    zoom : 8
  },
  "Dublin" : {
    lat : 53.399707,
    lng : -6.262207,
    zoom : 9
  }
};


$('#countyLocation').change(setLocationOnValChange);

// better to use a generic name in case function needs to be used on other
// event listeners (maybe a click event on another element)
function setLocationOnValChange (evt) {
  // no need to duplicate the selector, can use evt.currentTarget for it, also
  // makes function more flexible (can be used for other elements)
  setLocation( $(evt.currentTarget).val() );
}

// we split the setLocation logic since it might be needed in a different
// context (lets say you also want to trigger it after some data is loaded)
function setLocation (id) {
  var location = LOCATIONS[id];
  var center = new google.maps.LatLng(location.lat, location.lng);
  map.setCenter(center);
  map.setZoom(location.zoom);
}











// OPPORTUNITY: load data from the server
// - server-managed data!
// - return a promise -- more on that shortly

$('#countyLocation').change(setLocationOnValChange);

function setLocationOnValChange (evt) {
  setLocation( $(evt.currentTarget).val() );
}

function selectedLocation (id) {
  return $.get('/data/locations.json', function(locations) {
    var location = locations[id];
    var center = new google.maps.LatLng(location.lat, location.lng);
    map.setCenter(center);
    map.setZoom(location.zoom);
  });
}














// THE SMELL

function showPeople (people) {
  $.each(people, function (idx, person) {
    $('<li>', {
      id: 'person_' + person.id,
      text: person.name
    }).appendTo('#people');

    $('<button>', {
      id: 'button_' + person.id,
      text: 'Remove ' + person.name
    }).appendTo('#buttons');
  });

  $('#buttons button').on('click', function (e) {
    e.preventDefault();
    var id = $(this).attr('id').replace('button_', '');
    $('#person_' + id).remove();
  });
}













// THE FIX: take advantage of closures

function showPeople (people) {
  $.each(people, function (idx, person) {
    var li = $('<li>', {
      text: person.name
    }).appendTo('#people');

    $('<button>', {
      text: 'Remove ' + person.name,
      click: function( e ) {
        e.preventDefault();
        li.remove();
      }
    })
    .appendTo('#buttons');
  });
}
























// THE SMELL

function showPrice (row, standardPrice, discountPrice, savings, isProrated, isDeferred) {

  var priceCol = row.find('td.price').eq(0);
  var discountCol = row.find('td.price').eq(1);
  var txt = '';

  if (!discountPrice && !standardPrice && !savings && !isProrated && !isDeferred) {
    discountCol.html('<img src="/img/loading.gif" />');
  } else {
    if (discountPrice && (standardPrice == discountPrice)) {
      priceCol.html('');
    } else {
      priceCol.html('<del>' + standardPrice + '</del>');
    }

    txt = '<strong>';

    if (discountPrice == '$0.00') {
      txt += 'FREE!';
    }

    if (isProrated) {
      txt += '*';
    } else if (isDeferred) {
      txt += '**';
    }

    txt += '</strong>';
  }

  discountCol.html(txt);
}
































showPrice(row, null, null, null, null, null);















// THE FIX: more & smaller functions


// it's better to pass fewer arguments and use objects to set multiple options.
// a method should rarely have more than 2 arguments.
updatePriceRow(row, {
  discountPrice: '$0.00',
  isProrated: true
});


function updatePriceRow (row, priceInfo) {
  // we should not be passing HTML strings around, the API of our method should
  // receive plain objects and build the HTML by itself.
  row.find('td.price').html( getPriceHtml(priceInfo) );
  row.find('td.discount').html( getDiscountHtml(priceInfo) );
}


function getDiscountHtml (priceInfo) {
  var txt = '';

  if (priceInfo.discountPrice === '$0.00') {
    txt += 'FREE!';
  }

  if (priceInfo.isProrated) {
    txt += '*';
  }

  if (priceInfo.isDeferred) {
    txt += '**';
  }

  return txt ? ( '<strong>' + txt + '</strong>' ) :
    '<img src="/img/loading.gif" />';
}


function getPriceHtml (priceInfo) {
  if (!priceInfo.standardPrice) {
    return '';
  }

  if (priceInfo.standardPrice == priceInfo.discountPrice) {
    return '';
  }

  return '<del>' + priceInfo.standardPrice + '</del>';
}










// OPPORTUNITY: tests!

test('update price row', function () {
  var t = $('<table><tr><td class="price"/><td class="discount"/></tr></td></table>');
  var row = t.find('tr');

  udpatePriceRow(row, {
    standardPrice: 123,
    discountPrice: 110
  });

  assert.equal(row.find('td.price').html(), '<del>123</del>');
  assert.equal(row.find('td.discount').html(), '<strong>110</strong>');
});













// THE SMELL

function getSomeThings (callback) {
  var completed = 0;
  var people, tasks;

  $.ajax('/data/people.json', {
    dataType: 'json',
    success: function (data) {
      completed++;
      people = data.people;
      onFinished();
    }
  });

  $.ajax('/data/tasks.json', {
    dataType: 'json',
    success: function (data) {
      completed++;
      tasks = data.tasks;
      onFinished();
    }
  });

  function onFinished () {
    if (completed < 2) { return; }
    callback(people, tasks);
  }
}












// THE FIX: deferreds and promises

function getSomeThings (callback) {
  var people, tasks;

  var peopleRequest = $.ajax('/data/people.json', {
    dataType: 'json',
    success: function (data) {
      people = data.people;
    }
  });

  var taskRequest = $.ajax('/data/tasks.json', {
    dataType: 'json',
    success: function (data) {
      tasks = data.tasks;
    }
  });

  $.when(peopleRequest, taskRequest).done(function () {
    callback(people, tasks);
  });
}









// OPPORTUNITY: pre-processing your data, returning a promise

function getSomeThings () {
  var peopleRequest = $.getJSON('/data/people.json')
                        .then(function (resp) {
                          return resp.people;
                        });

  var taskRequest = $.getJSON( '/data/tasks.json' )
                        .then(function (resp) {
                          return resp.tasks;
                        });

  return $.when(peopleRequest, taskRequest);
}












// THE SMELL

function createTasksHtml (tasks) {

  $.each(tasks, function (entryIndex, entry) {
    var s_class;

    switch (this.status_id) {
      case 2:
        s_class = 'urgent';
        break;
      case 5:
        s_class = 'very_urgent';
        break;
      default:
        s_class = 'normal';
        break;
    }

    var tasks_ops = '<div id="d' + entryIndex + '" class="' + s_class + '"><input type="checkbox" id="c' + entryIndex + '" value="' + entryIndex + '" class="c_' + s_class + '"/> ' + this.task + '</div>';

    $('#tasks').append(tasks_ops);
  });
}













// THE FIX: templates

/*
<script type="text/template" id="tasks_template">
<% _.each(tasks, function(task) { %>

<div id="d<%= task.index %>" class="<% task.s_class %>">
  <input
    type="checkbox"
    id="c<%= task.index %>"
    value="<%= task.index %>"
    class="c_<%= task.s_class %>" >
  <%= task.task %>
</div>

<% }); %>
</script>
*/

function createTasksHtml (tasks) {
  var template = $('#tasks_template').text();

  tasks = $.map(tasks, function (entryIndex, task) {
    var classes = {
      2: 'urgent',
      5: 'very_urgent'
    };

    task.s_class = classes[entry.status_id] || 'normal';
    task.index = entryIndex;

    return task;
  });

  $('#tasks').append( _.template(tasks_template, tasks) );
}

// OPPORTUNITY: use AMD plugins to load the templates and keep as external
// files  that will get pre-compiled during build for better performance.
// (https://github.com/ZeeAgency/requirejs-tpl)

define(['exports', 'tpl!./tasks.html'], function(exports, template){

  exports.createTasksHtml = function (tasks) {

    tasks = $.map(tasks, function (entryIndex, task) {
      var classes = {
        2: 'urgent',
        5: 'very_urgent'
      };

      task.s_class = classes[entry.status_id] || 'normal';
      task.index = entryIndex;

      return task;
    });

    $('#tasks').append( template(tasks) );
  };

});













/*****************************************************************************
 *
 * Rebecca Murphey
 * Senior Software Engineer at Bazaarvoice
 * @rmurphey • rmurphey.com
 * bazaarvoice.com
 *
 * Repo:              github.com/rmurphey/js-minty-fresh
 * Links:             pinboard.in/u:rmurphey/t:js-minty-fresh/
 * On teh internets:  rmurphey.com/js-minty-fresh/presentation/
 *
 ****************************************************************************/

Decided to fork and refactor the "solutions" since I think they also contained code smells. It was an improvement from original but not as good as it could be (there is always room for improvements).