4
\$\begingroup\$

This code handles a response from an RSS feed. It organizes and appends to content. If there's a video embeded 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>
\$\endgroup\$
2
\$\begingroup\$

Late reply,

this code is good, I can only find 2 faults:

  1. Your code should have error handling for the JSON call, things can go wrong

  2. Too much horizontal stretching, I would introduce some sugar for this:

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

    could be

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

    Also, I find

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

    more readable.

As for find("*"), it seems to be cleanest way to access all children.

\$\endgroup\$

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.