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\$
1
  • 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, 2015 at 12:53

1 Answer 1

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\$
3
  • \$\begingroup\$ But when I run this code it throws me an error: TypeError: this.AjaxPoller is undefined \$\endgroup\$ Jun 17, 2015 at 5:25
  • 1
    \$\begingroup\$ Oops, had a typo in the poll method - fixed it now. \$\endgroup\$ Jun 18, 2015 at 11:48
  • \$\begingroup\$ Thanks. I was also thinking about using here Javascript module pattern. What do You think about it? \$\endgroup\$ Jun 18, 2015 at 11:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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