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

Window timeout alert

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
timeoutwindowalert

Problem

How can I improve this window timeout alert code? I need to add it to some third-party master page and don't want to add a separate file for it.

It must be JS only and IE-8 supported.

jsFiddle

(function () {
//    timedRefresh(10); //3600000
    delayedAlert();

    var timeoutID;

    function delayedAlert() {
        clearAlert();
      timeoutID = window.setTimeout(slowAlert, 10000);
    }

    function slowAlert() {
      alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
    }

    function clearAlert() {
      window.clearTimeout(timeoutID);
    }
})();

Solution

A few points to make about this:

(function () {
//    timedRefresh(10); //3600000
    delayedAlert();

    var timeoutID;

    function delayedAlert() {
        clearAlert();
      timeoutID = window.setTimeout(slowAlert, 10000);
    }

    function slowAlert() {
      alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
    }

    function clearAlert() {
      window.clearTimeout(timeoutID);
    }
})();


  • slowAlert/delayedAlert can be improved by moving slowAlert into delayedAlert as follows:



function delayedAlert() {
    clearAlert();
    timeoutID = window.setTimeout(function(){
        alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
    }, 10000);
}


  • I can't see why the clearTimeout call deserves its own function, when it only gets called from delayedAlert; You could just put it in delayedAlert:



function delayedAlert() {
    window.clearTimeout(timeoutID);
    timeoutID = window.setTimeout(function(){
        alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
    }, 10000);
}


-
You can make delayedAlert an anonymous function, so that it cannot be called other than inside that block by changing function delayedAlert() into var delayedAlert = function()

-
You call delayedAlert before timeoutID is defined:

delayedAlert();

var timeoutID;


If you're going to use globals (which are recommended against), declare them first, at the top.

(function(){
    var timeoutID;
    // functions and whatnot
    delayedAlert();


All resulting in:

(function(){
    var timeoutID;
    var delayedAlert = function() {
        window.clearTimeout(timeoutID);
        timeoutID = window.setTimeout(function(){
            alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
        }, 10000);
    }
    delayedAlert();
})();

Code Snippets

(function () {
//    timedRefresh(10); //3600000
    delayedAlert();

    var timeoutID;

    function delayedAlert() {
        clearAlert();
      timeoutID = window.setTimeout(slowAlert, 10000);
    }

    function slowAlert() {
      alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
    }

    function clearAlert() {
      window.clearTimeout(timeoutID);
    }
})();
function delayedAlert() {
    clearAlert();
    timeoutID = window.setTimeout(function(){
        alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
    }, 10000);
}
function delayedAlert() {
    window.clearTimeout(timeoutID);
    timeoutID = window.setTimeout(function(){
        alert("You may have been logged out due to inactivity. Click OK to refresh the page.");
    }, 10000);
}
delayedAlert();

var timeoutID;
(function(){
    var timeoutID;
    // functions and whatnot
    delayedAlert();

Context

StackExchange Code Review Q#101789, answer score: 5

Revisions (0)

No revisions yet.