Stack Overflow tab notifier user-script

Question

I searched for a notification script that shows inbox and rep change items on the browser tab in stackapps.com but either they didn't work or were outdated.

So, I've done this raw script myself and want to make it fool proof and robust: This is my first ever user script. So want to make it better. (Even if it wasn't the first, I would have wanted to make it better)

// ==UserScript== // @name Tab Notifier // @namespace http://2200ce.wordpress.com/ // @version 0.1 // @description A notification system that displays inbox and rep change notification on the tab // @author Amit Joki - http://stackoverflow.com/users/3001736/amit-joki // @include http://stackoverflow.com/* // @grant none // @require https://cdn.rawgit.com/kapetan/jquery-observe/master/jquery-observe.js // @match *://*.stackexchange.com/* // @match *://*.stackoverflow.com/* // @match *://*.superuser.com/* // @match *://*.serverfault.com/* // @match *://*.askubuntu.com/* // @match *://*.stackapps.com/* // @match *://*.mathoverflow.net/* // ==/UserScript== var userscript = function($) { $('.icon-inbox .unread-count, .icon-achievements .unread-count').observe({ attributes: true, attributeFilter: ['style'] }, function() { var title = document.title; var isInbox = $(this).parent().is(".icon-inbox") ? true : false; if (!$(this).attr('style')) { document.title = updater(title, $(this), isInbox); } else if ($(this).attr("style") == "display: none;") { debugger; var frags = title.replace(/• /,"•").split(/\s(?=\w)/); var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/); if(frags[0].indexOf("•") == -1){ title = frags[0].replace(rgx, "") + frags.slice(1).join(" "); } else { rgx = /^(\[)(\+\d+) •(\d+)(\])/; title = frags[0].replace(rgx, isInbox ? "$1$2$$4" : "$1$3$4"); } document.title = title + frags.slice(1).join(" "); } else { } }); function updater(title, elm, isInbox){ var txt = elm.text().trim(); var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/); if(title.indexOf("•") > -1){ rgx = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/; title = title.replace(rgx, "$1" + txt); } else if(rgx.test(title)){ title = title.replace(rgx, "[" + txt + "] "); } else {title = "[" + txt + "] " + title;} if(title.indexOf("[")> -1 && title.match(/\[/g).length == 2){ var arr = title.split(/\s(?=\w)/), rtn =['[',,' • ',,']']; arr[0].split(" ").forEach(function(e){ rtn[e.indexOf("+") > -1 ? 1 : 3] = e.replace(/\[|\]/g,""); }); title = rtn.join("") + " " + arr.slice(1).join(" "); } return title; } } var el = document.createElement('script'); el.type = 'text/javascript'; el.text = '(' + userscript + ')(jQuery);'; document.head.appendChild(el); 

What this does is when inbox item alone comes up, it shows as:

[{number of inbox item here}]

and same for rep change:

[+{rep change here}]

When both come, they will be displayed in the following style:

[+{rep change here} • {inbox items here}]

Also once the notifications are seen, it will be subsequently removed as well. I'm using MutationsObservers, which is warped by a plugin called jQuery.observe which looks to settle cross-browser issues.

How can this be improved?

Answer 1

Instead of this:

 var isInbox = $(this).parent().is(".icon-inbox") ? true : false; 

Since .is(...) is boolean, you can use its returned value directly without the ternary operator:

 var isInbox = $(this).parent().is(".icon-inbox"); 

---

This else is pointless, just delete it:

 } else { } 

---

The indentation is off in the else block, and the formatting doesn't follow common conventions:

 else if(rgx.test(title)){ title = title.replace(rgx, "[" + txt + "] "); } else {title = "[" + txt + "] " + title;} 

It would be better to write this code like this:

 else if(rgx.test(title)){ title = title.replace(rgx, "[" + txt + "] "); } else { title = "[" + txt + "] " + title; } 

---

In this code:

var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/); if (title.indexOf("•") > -1) { rgx = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/; title = title.replace(rgx, "$1" + txt); } // ... 

It's misleading that rgx is first assigned to something, then it's reassigned to something else inside the if branch. This makes the reader look suspiciously at the code after the if statement to see if rgx is used again for something.

Looking at the code, rgx is not used again. Which means, you're just reusing the variable inside the if branch. But it's not a good practice to reuse a variable for more than one purpose. It would be better to create a new variable inside the if branch, for example:

var rgx = (isInbox ? /\[\d+\]/ : /\[\+\d+\]/); if (title.indexOf("•") > -1) { var rgx2 = isInbox ? /^(\• )\d+/ : /^(\[)+\d+/; title = title.replace(rgx2, "$1" + txt); } 

Answer 2

Regarding your variables/function naming:

1. rgx, txt, arr: I knew what these abbreviations mean at first sight, but the point is they describe the technical type of what they contain and not the use-case semantics of their content.
2. I know elm stands for element (probably) and has nothing to do with one of these elms, but...

3. What are frags?

   • Fragging, deliberate killing of an unpopular member of one's own fighting unit, occasionally using a fragmentation grenade?
   • In deathmatch computer games, frag means to kill someone temporarily, originated from the military term?

4. What's a rtn? A return? A retaliation? ramblingThisNight?

5. el... see 2.
6. isInBox: What is in which box? Oh, there's nothing in a box. It's isIconInboxTheParent.
7. updater(...) is a noun.
   1. Functions do something with "nouns" hence their names should contain a verb.
   2. What does this update(...) update then? Perhaps updateBrowserTab(...)?