patternjavascriptMinor
Refreshing / Cycling through a parsed RSS feed every 5 mins
Viewed 0 times
rsscyclingeveryfeedminsthroughrefreshingparsed
Problem
I am working on a monitor signage display and have a "welcome to RSS" feed with just a title and description. I have code from feedEk that's been tweaked a bit to parse the feed and cycle it so I only have one title and desc. showing at a time. This feed could be added to or deleted info at any time so I need it to refresh every five minutes. I've tried several solutions on here and just can't seem to work it out.
Here is the adjusted FeedEk code with comments on the adjustments:
```
(function (e) {
e.fn.FeedEk = function (t) {
var n = {
FeedUrl: "http://myrss.com/",
MaxCount: 1,
ShowDesc: true,
ShowPubDate: false,
CharacterLimit: 100,
TitleLinkTarget: "_self",
iterate: false
};
if (t) {
e.extend(n, t)
}
var r = e(this).attr("id");
var i;
processFeedData = function (t) {
//This just makes it flash too much
//e("#" + r).empty();
var s = "";
en = t.responseData.feed.entries;
if (n.iterate == true) {
//Setting a variable to store current item
i = window.feedcur = typeof(window.feedcur) === 'undefined' ? 0 : window.feedcur;
t = en[i];
s = makeString(t);
//incrementing the current for the next time we loop through
window.feedcur = ((i+1)%en.length);
} else {
for (i=0;i' + s + "");
}
makeString = function (t) {
s = '' + t.title + "";
if (n.ShowPubDate) {
i = new Date(t.publishedDate);
s += '' + i.toLocaleDateString() + ""
}
if (n.ShowDesc) {
if (n.DescCharacterLimit > 0 && t.content.length > n.DescCharacterLimit) {
s += '' + t.content.substr(0, n.DescCharacterLimit) + "..."
} else {
s += '' + t.content + ""
}
}
return s;
}
if (typeof(window.feedContent) === 'undefined') {
Here is the adjusted FeedEk code with comments on the adjustments:
```
(function (e) {
e.fn.FeedEk = function (t) {
var n = {
FeedUrl: "http://myrss.com/",
MaxCount: 1,
ShowDesc: true,
ShowPubDate: false,
CharacterLimit: 100,
TitleLinkTarget: "_self",
iterate: false
};
if (t) {
e.extend(n, t)
}
var r = e(this).attr("id");
var i;
processFeedData = function (t) {
//This just makes it flash too much
//e("#" + r).empty();
var s = "";
en = t.responseData.feed.entries;
if (n.iterate == true) {
//Setting a variable to store current item
i = window.feedcur = typeof(window.feedcur) === 'undefined' ? 0 : window.feedcur;
t = en[i];
s = makeString(t);
//incrementing the current for the next time we loop through
window.feedcur = ((i+1)%en.length);
} else {
for (i=0;i' + s + "");
}
makeString = function (t) {
s = '' + t.title + "";
if (n.ShowPubDate) {
i = new Date(t.publishedDate);
s += '' + i.toLocaleDateString() + ""
}
if (n.ShowDesc) {
if (n.DescCharacterLimit > 0 && t.content.length > n.DescCharacterLimit) {
s += '' + t.content.substr(0, n.DescCharacterLimit) + "..."
} else {
s += '' + t.content + ""
}
}
return s;
}
if (typeof(window.feedContent) === 'undefined') {
Solution
I know this code is more than four years old and you haven't been on CR since then but maybe it will help somebody else.
String concatenation and adding HTML elements
The code adds a lot of HTML via string concatenation. It might be better to use the JS DOM element creation APIs like
String concatenation and adding HTML elements
The code adds a lot of HTML via string concatenation. It might be better to use the JS DOM element creation APIs like
document.createElement() or a template (e.g. with the ` tag).
Perhaps it is becoming less of a concern as browsers are being optimized, but it is wise to Minimize browser reflows. That means instead of adding elements to the DOM in a loop - e.g. in processFeedData(), add elements to a documentFragment or temporary element and then add the contents of that element in a single step.
Variable scope
There is a mix of local and global variables - for example, in processFeedData(), s is declared with the var keyword but then en is not, making it a global variable. Then in makeString() the variable s is assigned without having been declared locally so its scope is the wrapping function (i.e. e.fn.FeedEk = function (t) {...). It is best to avoid global variables to avoid unintentional consequences... For more in this topic refer to answers to why are globals bad in javascript.
Repeated DOM queries
The DOM-ready code in the second page looks up the element with id attribute divRss initially as well as in the interval callback. DOM lookups aren't cheap:
”...DOM access is actually pretty costly - I think of it like if I have a bridge - like two pieces of land with a toll bridge, and the JavaScript engine is on one side, and the DOM is on the other, and every time I want to access the DOM from the JavaScript engine, I have to pay that toll”
- John Hrvatin, Microsoft, MIX09, in this talk Building High Performance Web Applications and Sites at 29:38, also cited in the O'Reilly Javascript book by Nicholas C Zakas Pg 36, as well as mentioned in this post
Bearing in mind he said that ~10 years ago and browsers have come a long way, it is still wise to consider. I would recommend storing $('#divRss') to a variable (or constant using const` if supporting ecmascript-6) and referencing that variable instead of querying the DOM each time.Context
StackExchange Code Review Q#71605, answer score: 5
Revisions (0)
No revisions yet.