4
\$\begingroup\$

I wrote some ugly JavaScript that is responsible for sending Ajax requests and update the DOM. I'm using it to show user progress of background jobs.

this.AjaxPoller = {
  poll: function() {
    setTimeout(this.AjaxPoller.request, 5000);
  },

  request: function() {
    $(".ajax_poller[data-url]").each(function(i, elem) {
      url = $(elem).data("url");

      $.getJSON(url, function(data) {
        if (checkProgress(data)) {
          location.reload();
        };

        $.each(data, function( key, val ) {
          $(key+ ' .progress_info').html(val);
          $(key + ' .progress-bar').width(val);
        });
      });
    });
  }
};

var checkProgress = function(obj) {
  correct = true;

  for (key in obj) {
    if (obj[key] != '100%') correct = false;
  }
  return correct;
}

What can I refactor in this code to make it better?

\$\endgroup\$
  • 4
    \$\begingroup\$ Refactoring code is the goal of this site. Your title is general, please try to choose one that explain what the code do. Normally when you're here, it's for refactoring your code. \$\endgroup\$ – Marc-Andre Jun 16 '15 at 12:53
3
\$\begingroup\$

I personally like to rely on named functions rather than inline anonymous functions.

this.AjaxPoller = {
  poll: poll,
  request: request
};

function request() {
  $(".ajax_poller[data-url]").each(fetchDataForElement);
}

function fetchDataForElement(i, elem) {
  var url = $(elem).data("url");
  $.getJSON(url, handleJson);
}

function handleJson(data) {
  if (checkProgress(data)) {
    location.reload();
  }
  $.each(data, function(key, val) {
    $(key+ ' .progress_info').html(val);
    $(key + ' .progress-bar').width(val);
  });
}

function poll() {
  setTimeout(this.request, 5000); 
}

function checkProgress(obj) {
  var correct = true;
  for (key in obj) {
    if (obj[key] != '100%') correct = false;
  }
  return correct;
}
\$\endgroup\$
  • \$\begingroup\$ But when I run this code it throws me an error: TypeError: this.AjaxPoller is undefined \$\endgroup\$ – Mateusz Urbański Jun 17 '15 at 5:25
  • 1
    \$\begingroup\$ Oops, had a typo in the poll method - fixed it now. \$\endgroup\$ – Robert Messerle Jun 18 '15 at 11:48
  • \$\begingroup\$ Thanks. I was also thinking about using here Javascript module pattern. What do You think about it? \$\endgroup\$ – Mateusz Urbański Jun 18 '15 at 11:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.