Displaying RSS feeds from Google Feed API as HTML list

What it does is:

• Reads from a RSS feed using Google Feed API

• 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 ').append(
                            $(''+entry.title+'').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){
            $('')
                .css("color","red")
                .append("Error:"+errorText)
                .appendTo(FeedManager.config.feedContainer);
        }
    };

    FeedManager.init();
});

At the 2nd stage, I wanted to add custom accordion fe

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 ').append(
                $(''+entry.title+'').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 ').append(
            $(''+entry.title+'').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() {
        $('').append(
            $('' + this.title + '').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() {
        $('').append(
            $('').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() {
        $('').append(
            $('').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.

StackExchange Code Review Q#1118, answer score: 13