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?

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.

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