Displaying RSS feeds from Google Feed API as HTML list

Question

What it does is:

1. Reads from a RSS feed using Google Feed API
2. Shows the list in an unordered list

How good/bad is the code snippet?

$(document).ready(function(){
var FeedManager = {
 config : {
 feedContainer : $('#feedContainer'),
 feedUrl : 'http://rss.bdnews24.com/rss/english/home/rss.xml',
 feedLimit : 10
 }, 
 init: function(){
 var feed = new google.feeds.Feed(FeedManager.config.feedUrl);
 feed.setNumEntries(FeedManager.config.feedLimit) ;
 feed.load(function(result) {
 if (!result.error) {
 FeedManager.$feedContainer = FeedManager.config.feedContainer;
 for (var i = 0; i < result.feed.entries.length; i++) {
 var entry = result.feed.entries[i];
 $('<li/>').append(
 $('<a>'+entry.title+'</a>').attr(
 {
 'title': entry.title,
 'href': entry.link
 }
 ).bind('click',FeedManager.showStory)
 ).appendTo(FeedManager.$feedContainer);
 }
 }
 else{
 FeedManager.handleError(result.error.message);
 }
 });
 },
 showStory: function(){
 var href = event.currentTarget.href;
 FeedManager.showURL(href);
 event.preventDefault();
 },
 showURL: function(url){
 if (url.indexOf("http:") != 0 && url.indexOf("https:") != 0) {
 return;
 }
 chrome.tabs.create({
 url: url
 });
 },
 handleError: function(errorText){
 $('<li/>')
 .css("color","red")
 .append("Error:"+errorText)
 .appendTo(FeedManager.config.feedContainer);
 }
 };
 FeedManager.init();
});

At the 2nd stage, I wanted to add custom accordion feature and news snippet:

processFeedResult: function(result) {
 if (result.error) {
 FeedManager.handleError(result.error.message);
 return;
 }
 FeedManager.$feedContainer = FeedManager.config.feedContainer;
 $.each(result.feed.entries, function() {
 $('<li/>').append(
 $('<a>', { // this should not be an anchor tag,right? TBD 
 title: 'Published at: '+this.publishedDate,
 text: this.title
 })
 ).append($('<div>', {
 text: this.contentSnippet,
 css : {'display':'none',
 'padding-top':'2px'}
 }).append(
 $('<a>', {
 text: '[more..]',
 href: this.link,
 click: FeedManager.showStory
 })) 
 )
 .bind('click',FeedManager.showSnippet)
 .appendTo(FeedManager.$feedContainer);
 });
},
showSnippet: function() {
 var $obj = $(event.currentTarget),
 $snippetDiv = $obj.find('div').slideDown('normal');
 if(FeedManager.$lastOpenedDiv === undefined){
 FeedManager.$lastOpenedDiv = $snippetDiv ;
 }
 else{
 FeedManager.$lastOpenedDiv.slideUp('normal');
 FeedManager.$lastOpenedDiv = $snippetDiv ;
 }
 } 
};

I feel that I have put some tightly coupled code in my processFeedResult such as text and CSS. I also wanted to know if my showSnippet function good enough or not. However, it works, and I know that there are 3rd party good accordion available, but I wanted to learn it.

So far I've kept the anchor tag to show the news title and I used anchor title as a tooltip that shows the time. Maybe there is a good alternate, like span or paragraph?

• John Gietzen: I think that you should take a look at jQuery TMPL.
• zzzzBov: you should run it through jsLint. Also, you should use one var statement per function.

Answer 1

Looks ok, to be honest. A few minor changes I would make:

I would pull this code out into a function

function ProcessFeedResult(result) {
 if (!result.error) {
 FeedManager.$feedContainer = FeedManager.config.feedContainer;
 for (var i = 0; i < result.feed.entries.length; i++) {
 var entry = result.feed.entries[i];
 $('<li/>').append(
 $('<a>'+entry.title+'</a>').attr(
 {
 'title': entry.title,
 'href': entry.link
 }
 ).bind('click',FeedManager.showStory)
 ).appendTo(FeedManager.$feedContainer);
 }
 }
 else{
 FeedManager.handleError(result.error.message);
 }
}

Then I would reverse the condition and return, to reduce the level of indent a bit

function ProcessFeedResult(result) {
 if (result.error) {
 FeedManager.handleError(result.error.message);
 return;
 }
 FeedManager.$feedContainer = FeedManager.config.feedContainer;
 for (var i = 0; i < result.feed.entries.length; i++) {
 var entry = result.feed.entries[i];
 $('<li/>').append(
 $('<a>'+entry.title+'</a>').attr(
 {
 'title': entry.title,
 'href': entry.link
 }
 ).bind('click',FeedManager.showStory)
 ).appendTo(FeedManager.$feedContainer);
 }
}

Further, I would replace the loop with jQuery's $.each()

function ProcessFeedResult(result) {
 if (result.error) {
 FeedManager.handleError(result.error.message);
 return;
 }
 FeedManager.$feedContainer = FeedManager.config.feedContainer;
 $.each(result.feed.entries, function() {
 $('<li/>').append(
 $('<a>' + this.title + '</a>').attr(
 {
 'title': this.title,
 'href': this.link
 }
 ).bind('click',FeedManager.showStory)
 ).appendTo(FeedManager.$feedContainer);
 });
}

You can do any or all of these, I don't think any of it is vastly important. I can read your intent easily enough, it just looks a bit neater to me after refactoring.

Thinking about your later comment a bit, I would also be tempted to append the title, rather than stringing it together.

function ProcessFeedResult(result) {
 if (result.error) {
 FeedManager.handleError(result.error.message);
 return;
 }
 FeedManager.$feedContainer = FeedManager.config.feedContainer;
 $.each(result.feed.entries, function() {
 $('<li/>').append(
 $('<a/>').append(this.title).attr(
 {
 'title': this.title,
 'href': this.link
 }
 ).bind('click',FeedManager.showStory)
 ).appendTo(FeedManager.$feedContainer);
 });
}

Otherwise this is the correct approach to generating elements, as far as I know.

As a final step, I would tidy up the braces around the attributes, for consistency

function ProcessFeedResult(result) {
 if (result.error) {
 FeedManager.handleError(result.error.message);
 return;
 }
 FeedManager.$feedContainer = FeedManager.config.feedContainer;
 $.each(result.feed.entries, function() {
 $('<li/>').append(
 $('<a/>').append(this.title).attr({
 'title': this.title,
 'href': this.link
 }).bind('click',FeedManager.showStory)
 ).appendTo(FeedManager.$feedContainer);
 });
}

I really don't think you're going to get it any cleaner than that.

• Shahriar: Thanks for the nice help! No worries,The code worked fine :D , Still I am wondering, how to build the li and anchor tags in a nice way.
• pdr: @user1234 - See latest edit
• pdr: @nezz - Happy to help, it was a good question. Don't forget to vote and accept on the left
• Shahriar: sure,but I still don't have 15 reputation to qualify for voting :(
• Shahriar: @pdr - I've added some new features,would you give a look please.

Answer 2

You can use jQuery 1.4's new element creation syntax to shorten things a little (rewriting pdr's answer):

$('<li>').append(
 $('<a>', {
 title: this.title,
 text: this.title,
 href: this.link,
 click: FeedManager.showStory
 })
).appendTo(FeedManager.$feedContainer);

And the same can be done with the element created in the handleError function

Answer 3

There is no comments.

This:

What it does is:

Reads from a RSS feed using google feed API shows the list in an unordered list.

Should be a comment somewhere in top of the code