Skip to main content
added 74 characters in body
Source Link
Joseph
  • 25.2k
  • 2
  • 25
  • 37

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

  • Using what I call "return early". That way, it avoids nested ifs and your code looks flat.

  • Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.

  • Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.

  • Replaced your loop with reduce because it was designed for that, an aggregator of values.

  • You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat. If the string is potentially not a number, guard yourself with isNan.

  • Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?

  • Notification title moved down nearer to notification and after the two early-return if. That way, you don't actually generate the title if the script bails out on either of the two returns.

  • Inlined your notification details object. Assigning to a variable just took up space, only to be used once.

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewCountreviewItemsArray = Array.prototype.slice.call(reviewItems);
var reviewCount = reviewItemsArray.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

  • Using what I call "return early". That way, it avoids nested ifs and your code looks flat.

  • Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.

  • Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.

  • Replaced your loop with reduce because it was designed for that, an aggregator of values.

  • You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat. If the string is potentially not a number, guard yourself with isNan.

  • Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?

  • Notification title moved down nearer to notification and after the two early-return if. That way, you don't actually generate the title if the script bails out on either of the two returns.

  • Inlined your notification details object. Assigning to a variable just took up space, only to be used once.

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewCount = reviewItems.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

  • Using what I call "return early". That way, it avoids nested ifs and your code looks flat.

  • Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.

  • Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.

  • Replaced your loop with reduce because it was designed for that, an aggregator of values.

  • You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat. If the string is potentially not a number, guard yourself with isNan.

  • Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?

  • Notification title moved down nearer to notification and after the two early-return if. That way, you don't actually generate the title if the script bails out on either of the two returns.

  • Inlined your notification details object. Assigning to a variable just took up space, only to be used once.

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewItemsArray = Array.prototype.slice.call(reviewItems);
var reviewCount = reviewItemsArray.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);
added 310 characters in body
Source Link
Joseph
  • 25.2k
  • 2
  • 25
  • 37

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

  • Using what I call "return early". That way, it avoids nested ifs and your code looks flat.

  • Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.

  • Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.

  • Replaced your loop with reduce because it was designed for that, an aggregator of values.

  • You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat. If the string is potentially not a number, guard yourself with isNan.

  • Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?

  • Notification title moved down nearer to notification and after the two early-return if. That way, you don't actually generate the title if the script bails out on either of the two returns.

  • Inlined your notification details object. Assigning to a variable just took up space, only to be used once.

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewCount = reviewItems.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

  • Using what I call "return early". That way, it avoids nested ifs and your code looks flat.

  • Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.

  • Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.

  • Replaced your loop with reduce because it was designed for that, an aggregator of values.

  • You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat.

  • Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewCount = reviewItems.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

  • Using what I call "return early". That way, it avoids nested ifs and your code looks flat.

  • Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.

  • Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.

  • Replaced your loop with reduce because it was designed for that, an aggregator of values.

  • You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat. If the string is potentially not a number, guard yourself with isNan.

  • Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?

  • Notification title moved down nearer to notification and after the two early-return if. That way, you don't actually generate the title if the script bails out on either of the two returns.

  • Inlined your notification details object. Assigning to a variable just took up space, only to be used once.

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewCount = reviewItems.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);
Source Link
Joseph
  • 25.2k
  • 2
  • 25
  • 37

Haven't actually tested the code, just writing JavaScript from the top of my head. Here's some notable changes:

  • Using what I call "return early". That way, it avoids nested ifs and your code looks flat.

  • Renamed a few variables so that they have more meaning, especially DELAY which is very generic and because you have 2 timeouts. Long names don't matter, readability should always be considered.

  • Instead of using Math.abs, why not reverse the equation? Current time is always larger than time in the past. You'll always get a positive value.

  • Replaced your loop with reduce because it was designed for that, an aggregator of values.

  • You can always use the unary + as a shorthand for converting strings into numbers. In most cases, it acts similar to parseFloat.

  • Replaced your currentTime to just Date.now(). By now, browsers should at least be ES5-compliant. Anything less... you still rolling with IE6 or sumthin'?

And the end result:

Notification.requestPermission();

var KEY_NEXT = 'NextReload';
var NOTIFICATION_AUTODISMISS_TIMEOUT = 100000;
var RELOAD_DELAY =  120 * 1000; //120,000 milliseconds = 2 Minutes

var currentTime = Date.now();
var lastTime = GM_getValue(KEY_NEXT, 0);
var timeDiff = currentTime - lastTime;
var nextTime = currentTime + RELOAD_DELAY;

GM_setValue(KEY_NEXT, nextTime);

setTimeout(window.location.reload, RELOAD_DELAY);

if (timeDiff > RELOAD_DELAY * 2) return;

var reviewItems = document.getElementsByClassName('dashboard-num');
var reviewCount = reviewItems.reduce(function(count, reviewItem){
    return +reviewItem.getAttribute('title').replace(',', '') + count;
}, 0);

if (reviewCount <= 0)  return;

var notificationTitle = (document.title.split(' - ')[1] + ' Review Queue').replace(' Stack Exchange', '.SE');
var n = new Notification(notificationTitle, {
    body: reviewCount + ' Review Items',
    icon: 'https://github.com/malachi26/ReviewQueueNotifier/raw/master/Resources/Icon2.jpg'
});

n.onclick = function(){
    window.focus();
    this.cancel();
};

setTimeout(n.close.bind(n), NOTIFICATION_AUTODISMISS_TIMEOUT);