Skip to main content
Code Review

Return to Answer

Bounty Awarded with 50 reputation awarded by RubberDuck
improved clarity
Source Link
Quill
Quill
  • 12.1k
  • 5
  • 41
  • 94

You don't use title for anything other than title[1], so just use var title = document.title.split(' - ')[1] instead.

As of ECMAScript 5, the default radix used in parseInt is supposed to be \$10\$, before that, \$0\$ would get parsed as a octal number instead of a decimal number, and, as it seems, you can just omit that optional radix argument entirely. Although, as @EthanBierlein pointed out in the comments; MDN recommends not to omit it.

You don't use title for anything other than title[1], so just use .split(' - ')[1] instead.

As of ECMAScript 5, the default radix used in parseInt is supposed to be \$10\$, before that, \$0\$ would get parsed as a octal number instead of a decimal number, and, as it seems, you can just omit that optional radix argument entirely.

You don't use title for anything other than title[1], so just use var title = document.title.split(' - ')[1] instead.

As of ECMAScript 5, the default radix used in parseInt is supposed to be \$10\$, before that, \$0\$ would get parsed as a octal number instead of a decimal number, and, as it seems, you can just omit that optional radix argument entirely. Although, as @EthanBierlein pointed out in the comments; MDN recommends not to omit it.

Source Link
Quill
Quill
  • 12.1k
  • 5
  • 41
  • 94

This is mostly personal preference, but, I usually stack the @matches in descending size:

// @match *://*.stackexchange.com/review*
// @match *://*.stackoverflow.com/review*
// @match *://*.superuser.com/review*
// @match *://*.serverfault.com/review*
// @match *://*.askubuntu.com/review*
// @match *://*.stackapps.com/review*
// @match *://*.mathoverflow.net/review*

into:

// @match *://*.stackexchange.com/review*
// @match *://*.stackoverflow.com/review*
// @match *://*.mathoverflow.net/review*
// @match *://*.serverfault.com/review*
// @match *://*.askubuntu.com/review*
// @match *://*.stackapps.com/review*    
// @match *://*.superuser.com/review*

Just looks nicer, I suppose.

var title = document.title.split(' - ');:

You don't use title for anything other than title[1], so just use .split(' - ')[1] instead.

.replace(',',''): You should have a space after ','.

You could consider replacing the .length style for loop with a for (var i in reviewItems) style loop. If not, you can also declare reviewCount in the for loop like: for (var reviewCount = 0, i = 0;.

var details = {
        title: document.title,

Desktop Notifications - Code Review Stack Exchange sounds a little bulky and over-the-top. Review Items is a better name for it, and you could consider removing Stack Exchange, but, consider the effect that would have on MSE.

I can't see the point in using notifications as an array, as it should really just have the one notification.

if (reviewCount > 0) {
    notifications.push(reviewCount + ' Review Items');
}
if (notifications.length) {
    var details = {
        text: notifications.join('\n'),

if (reviewCount > 0) {
    notification = reviewCount + ' Review Items';
}
if (notification) {
    var details = {
        text: notification,

or even just move reviewCount + ' Review Items' into details, like:

if (reviewCount > 0) {
    var details = {
        text: reviewCount + ' Review Items',

As of ECMAScript 5, the default radix used in parseInt is supposed to be \$10\$, before that, \$0\$ would get parsed as a octal number instead of a decimal number, and, as it seems, you can just omit that optional radix argument entirely.