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>
asked Jul 23, 2013 at 19:53
\$\endgroup\$

1 Answer 1

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.

answered Apr 28, 2014 at 14:00
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.