Jonny Sooter
Jonny Sooter

Reputation: 2417

Improve AJAX response handling function

This code handles a response from an RSS feed. It organizes and appends to content. If there's a video embedded then separate it from the rest of the content. I would like a review mainly on performance/efficiency but I'm open to any other suggestions as well. That star selector is really nagging me but I don't know of a better way to iterate over all the contained elements.

function getFeed(url, element, callback) {
    $.getJSON("https://ajax.googleapis.com/ajax/services/feed/load?v=1.0&callback=?&q="+encodeURIComponent(url), function(response) {
        var content = "",
            $element = $(element);

        for (var i = 0; i < response.responseData.feed.entries.length; i++) {
            content = content + response.responseData.feed.entries[i].content; //Join all the feed entries
        }

        $element.find(".content").html(content).addClass($element.find("embed").length? "withVideo" : "");

        $element.find("*").each(function() {
            var $this = $(this);

            $this.removeAttr("style width align"); //Reset all the crap that comes with the response

            if ($this.is("embed")) {
                $element.append("<div class='video'></div>");
                $this.attr("width", 640).attr("height", 360).parent().appendTo(element + " .video");
            };
        });

        if (typeof callback === 'function') {
            callback();
        };
    });
}

This is then called like so:

getFeed("http://www.kent.k12.wa.us/site/RSS.aspx?PageID=3854", "#TechExpo", optionalCallback);

Here's what a response might look like

<div width="500" style="whatever"><p>Some text blah blah blah.</p>
<p align="right">Some more text</p>
</div>
<div><h2>Video Title</h2>
<embed src="http://..." width="360" height="202" type="application/x-shockwave-flash"></embed>
<small>Watch the 7th annual Tech Expo highlights.</small></div>

Upvotes: 1

Views: 93

Answers (1)

Webberig
Webberig

Reputation: 2806

First: Don't use ".length" inside a for statement. That way it will count the number of items during every pass in the loop.

var responseCount = response.responseData.feed.entries.length;
for (var i = 0; i < responseCount, i++) {
...
}

Secondly, I'm not sure this is a very good idea (performance-wise at least):

$element.find("*")

You definately can optimize this!

Performance/efficiency usually goes further then just going over a single function. If not used properly, jQuery can cause a large performance hit. Depending on the scope of your project, you could try the following:

  • Consider version jQuery 2.* which is much smaller and faster (but lacks backwards compatibility)
  • Consider using a custom build of jQuery.
  • do the browsers in your application's scope support querySelector ? If so, you probably don't even need jQuery...
  • Consider lazyloading your code scripts to speed up loading time of your page

This website contains a pretty good checklist for further reading: http://browserdiet.com/#js

Upvotes: 1

Related Questions