What's causing these javascript validation errors (Implied global and already defined var)?

Question

Could anyone explain me why:

function doAjax() {
    var xmlHttpReq = false;
    try { // Firefox, Opera 8.0+ and Safari
     xmlHttpReq = new XMLHttpRequest();
    }
    catch (e) { // Internet Explorer
     try {
      xmlHttpReq = new ActiveXObject("Msxml2.XMLHTTP");
     }
     catch (e) {
      try {
       xmlHttpReq = new ActiveXObject("Microsoft.XMLHTTP");
      }
      catch (e) {
       alert("Your browser does not support AJAX. Please use an AJAX compatible browser.");
       return false;
      }
     }
    }
    xmlHttpReq.open('GET', 'handler.php', true);
    xmlHttpReq.onreadystatechange = function() {
     if (xmlHttpReq.readyState == 4) {
      var response = xmlHttpReq.responseText;
      handleAjaxResponse(response);
     }
    };
    xmlHttpReq.send(null);
    return true;
}

is causing the following validation errors:

Error:

Implied global: ActiveXObject 8, XMLHttpRequest 4, alert 15, handleAjaxResponse 24

Problem at line 10 character 16: 'e' is already defined.

catch (e) {

Problem at line 14 character 20: 'e' is already defined.

catch (e) {

by the JSlint.com javascript validator

Answer 1

You are reusing the variable e in each try/catch block. Try renaming them to avoid the collision. The other issues are just warnings that you are using things that need to be defined elsewhere.

Answer 2

JSlint usually give a LOT of errors...

'e' is already defined seems pretty clear to me:) you use the same variable for all your try-catch statements.

Answer 3

Regarding the first error, here is an exerpt from the JSLint Documentation:

Undefined Variables and Functions

JavaScript's biggest problem is its dependence on global variables, particularly implied global variables. If a variable is not explicitly declared (usually with the var statement), then JavaScript assumes that the variable was global. This can mask misspelled names and other problems.

JSLint expects that all variables and functions are declared before they are used or invoked. This allows it to detect implied global variables. It is also good practice because it makes programs easier to read.

Sometimes a file is dependent on global variables and functions that are defined elsewhere. You can identify these to JSLint by including a comment in your file that lists the global functions and objects that your program depends on, but that are not defined in your program or script file.

A global declaration can look like this:

/*global getElementByAttribute, breakCycles, hanoi */

A global declaration starts with /*global. Notice that there is no space before the g. You can have as many /*global comments as you like. They must appear before the use of the variables they specify.

Regarding your problem, the following section is most likely to help you fix the errors:

Some globals can be predefined for you. Select the Assume a browser (browser) option (see Options below) to predefine the standard global properties that are supplied by web browsers, such as window and document and alert. Select the Assume Rhino (rhino) option to predefine the global properties provided by the Rhino environment. Select the Assume a Yahoo Widget (widget) option to predefine the global properties provided by the Yahoo! Widgets environment.

The second error is given because you re-use the variable "e" for each exception, including the nested ones. Rename the variables on each exception to avoid this.

Answer 4

It would be more sensible to use framework like jQuery (especially if you seriously want to support older versions of IE (pre v6) ) but I'll assume there is a reason you're not doing that.

It would be better if a) you don't nest try-catches and b) you factored out a set of functions namely one to get an Xhr object, another to use an Xhr object to make a generic ajax request and an outer "doAjax" function that performs the specific ajax call you want to make:-

function getXHR()
{
    var result = null
    if (window.XMLHttpRequest)
    {
        result = new XMLHttpRequest();
    }
    else
    {
        try { result = new ActiveXObject("MSXML2.XMLHTTP.3.0") }
        catch (e) { }

        if (result == null)
        {
            try { result = new ActiveXObject("Microsoft.XMLHTTP") }
            catch (e) { }
        }
    }
    return result; 
}


function ajaxRequest(url, data, callBack)
{
    var xmlHttpReq = getXHR();
    if (xmlHttpReq)
    {
        xmlHttpReq.open(data != null ? 'GET' : 'POST', url, true);
        xmlHttpReq.onreadystatechange = function()
        {
            if (xmlHttpReq.readyState == 4)
            {
                //what happens if status is not 200
                callBack(xmlHttpReq.responseText);
            }
        };
        xmlHttpReq.send(null);
        return true;
    }
    else
    {
        return false;
    }
}

function doAjax()
{
     var result = ajaxRequest('handler.php', null, handleAjaxResponse);
     if (!result) alert("Your browser does not support AJAX. Please use an AJAX compatible browser.");
     return result;
}

A futher refinement would be to make the callback accept an XHR object rather than the basic responseText. This would give you more flexibility. If the callback function simply wants the text it can use this function:-

function getTextFromXhr(xhr)
{
    xhr.onreadystatechange = fnVoid;
    if (xhr.status == 200)
    {
        return xhr.responseText;
    }
    else
    {
        throw {number: xhr.status,
            description: xhr.statusText,
            responseText: xhr.responseText
        }
    }
}