HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

IGB Stack Flair – A dynamic flair for Stack Exchange sites

Submitted by: @import:stackexchange-codereview··
0
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 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...

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.