patternjavascriptMinor
Improve AJAX response handling function
Viewed 0 times
handlingajaxresponsefunctionimprove
Problem
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.
This is then called like so:
Here's what a response might look like
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 ");
$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
Some text blah blah blah.
Some more text
Video Title
Watch the 7th annual Tech Expo highlights.Solution
Late reply,
this code is good, I can only find 2 faults:
-
Your code should have error handling for the JSON call, things can go wrong
-
Too much horizontal stretching, I would introduce some sugar for this:
could be
Also, I find
more readable.
As for
this code is good, I can only find 2 faults:
-
Your code should have error handling for the JSON call, things can go wrong
-
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.Code Snippets
for (var i = 0; i < response.responseData.feed.entries.length; i++) {
content = content + response.responseData.feed.entries[i].content; //Join all the feed entries
}var entries = response.responseData.feed.entries;
for (var i = 0; i < entries.length; i++) {
content = content + entries[i].content; //Join all the feed entries
}$element.find(".content").html(content)
.addClass($element.find("embed").length? "withVideo" : "");Context
StackExchange Code Review Q#28896, answer score: 2
Revisions (0)
No revisions yet.