patternjavascriptMinor
IGB Stack Flair – A dynamic flair for Stack Exchange sites
Viewed 0 times
stackexchangesitesigbflairfordynamic
Problem
I created the Stackapp IGB Stack Flair a while ago and I think it's a good time for a revision now. Is there anything that can be done better, shorter, more elegant, more performant, etc.?
I thought of using something like a map/associative array for the SE sites in
Please note:
```
/*
* IGB Stack Flair – A dynamic flair for Stack Exchange sites
* Copyright (C) 2017 Gerold Broser (geribro@users.sourceforge.net)
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see .
*/
/*
* FIXME:
* - use jQuery
* - cookie caching doesn't seem to work, perhaps a Firefox add-on issue
* TODO:
* - test in browsers other than Firefox, Opera, Chrome
* - add further SE sites to 'sites' prior to 'addSiteLinkFor()'
* Future:
* - add i18n
*/
let usersAJAX = null
let userAJAX = null
let users = null
let seUser = null // had to be renamed from 'user' since otherwise it conflicts with Wikipedia
let cookieExpirationTime = null
let usingCachedInfo = true
/** A dynamic flair for Stack Exchange sites inspired b
I thought of using something like a map/associative array for the SE sites in
addSiteLinkFor(), for instance. [This has been implemented. --G.B., April 21, 2017]Please note:
- My indent style is Whitesmiths adapted by the opening brace on the same line to save vertical space here.
- The spaces after/before opening/closing parenthesis are intentional.
- 120 chars line length are intentional (I can share a GreaseMonkey script that overcomes this width limit of SE's sites if you want.)
```
/*
* IGB Stack Flair – A dynamic flair for Stack Exchange sites
* Copyright (C) 2017 Gerold Broser (geribro@users.sourceforge.net)
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see .
*/
/*
* FIXME:
* - use jQuery
* - cookie caching doesn't seem to work, perhaps a Firefox add-on issue
* TODO:
* - test in browsers other than Firefox, Opera, Chrome
* - add further SE sites to 'sites' prior to 'addSiteLinkFor()'
* Future:
* - add i18n
*/
let usersAJAX = null
let userAJAX = null
let users = null
let seUser = null // had to be renamed from 'user' since otherwise it conflicts with Wikipedia
let cookieExpirationTime = null
let usingCachedInfo = true
/** A dynamic flair for Stack Exchange sites inspired b
Solution
Take a look at these two snippets. This one...
and
that one. Compare the indentation.
You're not being consistent with indentation. You've gone for a style I've never seen used before - fine by me. But if you don't follow it up by being consistent, it just hurts the code. Get a code formatter that can format the code in the way you prefer consistently, and use it.
Oh, by the way...
That's a double space.
Specifically:
vs
Is about your braces, and this...
vs
You're not being consistent with spaces for if-statements.
And your indentation style is dangerous:
snip...
```
if (seUser === null) {
let userURL = 'https://api.stackexchange.com/2.2/users/' + users.items[0].user_id + '?site=' +
users.items[0].site_url.substring(7) + '&filter=!*MxNUg6fdlYu_0rC&'
userAJAX.open( 'GET', userURL, true)
userAJAX.onreadystatechange = function() {
if ( this.readyState == 4 ) {
if ( this.status == 200 ) {
//snip
if ( siteUser.site_url.indexOf('meta') == 8 ) {
siteImage.src = 'https://meta.stackexchange.com/content/Sites/stackexchangemeta/img/icon-16.png'
siteImage.alt = sites['meta'].short
}
else if (siteUser.site_url.indexOf('stackoverflow') == 7 ) {
siteImage.src = 'https://cdn.sstatic.net/Sites/stackoverflow/img/icon-16.png'
siteImage.alt = sites['stackoverflow'].short
}
else {
siteServer = siteUser.site_url.substr( 8, siteUser.site_url.indexOf('.') - 8 )
siteImage.src = 'https://cdn.sstatic.net/Sites/' + siteServer + '/img/icon-16.png'
siteImage.alt = sites[siteServer].short
}and
if (seUser === null) {
let userURL = 'https://api.stackexchange.com/2.2/users/' + users.items[0].user_id + '?site=' +
users.items[0].site_url.substring(7) + '&filter=!*MxNUg6fdlYu_0rC&'
userAJAX.open( 'GET', userURL, true)
userAJAX.onreadystatechange = function() {
if ( this.readyState == 4 ) {
if ( this.status == 200 ) {
seUser = JSON.parse(this.responseText).items[0]
document.cookie = "stackUser" + networkProfileId + "=" + encodeURIComponent(JSON.stringify(user)) +
";expires=" + cookieExpirationTime
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
} // if ( status OK )
else {
console.error( "igbstackflair.js: " + userURL + "\n" + this.statusText + ": " + this.responseText )
document.getElementById( elementId )
.innerHTML = "igbstackflair.js: " + userURL + "" + this.statusText + ": " + this.responseText
}
} // if (status ready)
} // onreadystatechange function()
userAJAX.send( null )
}
else { // seUser !== null
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
}that one. Compare the indentation.
You're not being consistent with indentation. You've gone for a style I've never seen used before - fine by me. But if you don't follow it up by being consistent, it just hurts the code. Get a code formatter that can format the code in the way you prefer consistently, and use it.
Oh, by the way...
else {That's a double space.
Specifically:
if ( siteUser.site_url.indexOf('meta') == 8 ) {
siteImage.src = 'https://meta.stackexchange.com/content/Sites/stackexchangemeta/img/icon-16.png'
siteImage.alt = sites['meta'].short
}vs
if ( this.status == 200 ) {
seUser = JSON.parse(this.responseText).items[0]
document.cookie = "stackUser" + networkProfileId + "=" + encodeURIComponent(JSON.stringify(user)) +
";expires=" + cookieExpirationTime
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
} // if ( status OK )Is about your braces, and this...
else if (siteUser.site_url.indexOf('stackoverflow') == 7 ) {vs
if ( this.status == 200 ) {You're not being consistent with spaces for if-statements.
And your indentation style is dangerous:
if (seUser === null) {
let userURL = 'https://api.stackexchange.com/2.2/users/' + users.items[0].user_id + '?site=' +
users.items[0].site_url.substring(7) + '&filter=!*MxNUg6fdlYu_0rC&'
userAJAX.open( 'GET', userURL, true)
userAJAX.onreadystatechange = function() {
if ( this.readyState == 4 ) {
if ( this.status == 200 ) {
seUser = JSON.parse(this.responseText).items[0]
document.cookie = "stackUser" + networkProfileId + "=" + encodeURIComponent(JSON.stringify(user)) +
";expires=" + cookieExpirationTime
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
} // if ( status OK )
else {
console.error( "igbstackflair.js: " + userURL + "\n" + this.statusText + ": " + this.responseText )
document.getElementById( elementId )
.innerHTML = "igbstackflair.js: " + userURL + "" + this.statusText + ": " + this.responseText
}
} // if (status ready)
} // onreadystatechange function()
userAJAX.send( null )
}
else { // seUser !== null
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
}snip...
```
if (seUser === null) {
let userURL = 'https://api.stackexchange.com/2.2/users/' + users.items[0].user_id + '?site=' +
users.items[0].site_url.substring(7) + '&filter=!*MxNUg6fdlYu_0rC&'
userAJAX.open( 'GET', userURL, true)
userAJAX.onreadystatechange = function() {
if ( this.readyState == 4 ) {
if ( this.status == 200 ) {
//snip
Code Snippets
if ( siteUser.site_url.indexOf('meta') == 8 ) {
siteImage.src = 'https://meta.stackexchange.com/content/Sites/stackexchangemeta/img/icon-16.png'
siteImage.alt = sites['meta'].short
}
else if (siteUser.site_url.indexOf('stackoverflow') == 7 ) {
siteImage.src = 'https://cdn.sstatic.net/Sites/stackoverflow/img/icon-16.png'
siteImage.alt = sites['stackoverflow'].short
}
else {
siteServer = siteUser.site_url.substr( 8, siteUser.site_url.indexOf('.') - 8 )
siteImage.src = 'https://cdn.sstatic.net/Sites/' + siteServer + '/img/icon-16.png'
siteImage.alt = sites[siteServer].short
}if (seUser === null) {
let userURL = 'https://api.stackexchange.com/2.2/users/' + users.items[0].user_id + '?site=' +
users.items[0].site_url.substring(7) + '&filter=!*MxNUg6fdlYu_0rC&'
userAJAX.open( 'GET', userURL, true)
userAJAX.onreadystatechange = function() {
if ( this.readyState == 4 ) {
if ( this.status == 200 ) {
seUser = JSON.parse(this.responseText).items[0]
document.cookie = "stackUser" + networkProfileId + "=" + encodeURIComponent(JSON.stringify(user)) +
";expires=" + cookieExpirationTime
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
} // if ( status OK )
else {
console.error( "igbstackflair.js: " + userURL + "\n" + this.statusText + ": " + this.responseText )
document.getElementById( elementId )
.innerHTML = "igbstackflair.js: " + userURL + "<br />" + this.statusText + ": " + this.responseText
}
} // if (status ready)
} // onreadystatechange function()
userAJAX.send( null )
}
else { // seUser !== null
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
}if ( siteUser.site_url.indexOf('meta') == 8 ) {
siteImage.src = 'https://meta.stackexchange.com/content/Sites/stackexchangemeta/img/icon-16.png'
siteImage.alt = sites['meta'].short
}if ( this.status == 200 ) {
seUser = JSON.parse(this.responseText).items[0]
document.cookie = "stackUser" + networkProfileId + "=" + encodeURIComponent(JSON.stringify(user)) +
";expires=" + cookieExpirationTime
addLinks( networkProfileLink, maxSiteLinksCount, maxSiteLinksPerLine )
} // if ( status OK )else if (siteUser.site_url.indexOf('stackoverflow') == 7 ) {Context
StackExchange Code Review Q#160812, answer score: 2
Revisions (0)
No revisions yet.