views:

1281

answers:

8

We all know that global variables are anything but best practice. But there are several instances when it is difficult to code without them. What techniques do you use to avoid the use of global variables?

For example, given the following scenario, how would you not use a global variable?

Javascript:

var uploadCount = 0;

window.onload = function() {
  var frm = document.forms[0];

  frm.target = "postMe";
  frm.onsubmit = function() {
    startUpload();
    return false;
  }
}

function startUpload() {
  var fil = document.getElementById("FileUpload" + uploadCount);

  if(!fil || fil.value.length == 0) {    
    alert("Finished!");
    document.forms[0].reset();
    return;
  }

  disableAllFileInputs();
  fil.disabled = false;
  alert("Uploading file " + uploadCount);
  document.forms[0].submit();
}

Relevant markup:

<iframe src="test.htm" name="postHere" id="postHere"
  onload="uploadCount++; if(uploadCount > 1) startUpload();"></iframe>

<!-- MUST use inline Javscript here for onload event
     to fire after each form submission -->

This code comes from a web form with multiple <input type="file">. It uploads the files one at a time to prevent huge requests. It does this by POSTing to the iframe, waiting for the response which fires the iframe onload, and then triggers the next submission.

You don't have to answer this example specifically, I am just providing it for reference to a situation in which I am unable to think of a way to avoid global variables.

+7  A: 

The easiest way is to wrap your code in a closure and manually expose only those variables you need globally to the global scope:

(function() {
    // Your code here

    // Expose to global
    window['varName'] = varName;
})();

To address Crescent Fresh's comment: in order to remove global variables from the scenario entirely, the developer would need to change a number of things assumed in the question. It would look a lot more like this:

Javascript:

(function() {
    var addEvent = function(element, type, method) {
        if('addEventListener' in element) {
            element.addEventListener(type, method, false);
        } else if('attachEvent' in element) {
            element.attachEvent('on' + type, method);

        // If addEventListener and attachEvent are both unavailable,
        // use inline events. This should never happen.
        } else if('on' + type in element) {
            // If a previous inline event exists, preserve it. This isn't
            // tested, it may eat your baby
            var oldMethod = element['on' + type],
                newMethod = function(e) {
                    oldMethod(e);
                    newMethod(e);
                };
        } else {
            element['on' + type] = method;
        }
    },
        uploadCount = 0,
        startUpload = function() {
            var fil = document.getElementById("FileUpload" + uploadCount);

            if(!fil || fil.value.length == 0) {    
                alert("Finished!");
                document.forms[0].reset();
                return;
            }

            disableAllFileInputs();
            fil.disabled = false;
            alert("Uploading file " + uploadCount);
            document.forms[0].submit();
        };

    addEvent(window, 'load', function() {
        var frm = document.forms[0];

        frm.target = "postMe";
        addEvent(frm, 'submit', function() {
            startUpload();
            return false;
        });
    });

    var iframe = document.getElementById('postHere');
    addEvent(iframe, 'load', function() {
        uploadCount++;
        if(uploadCount > 1) {
            startUpload();
        }
    });

})();

HTML:

<iframe src="test.htm" name="postHere" id="postHere"></iframe>

You don't need an inline event handler on the <iframe>, it will still fire on each load with this code.

Regarding the load event

Here is a test case demonstrating that you don't need an inline onload event. This depends on referencing a file (/emptypage.php) on the same server, otherwise you should be able to just paste this into a page and run it.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"&gt;
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
    <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/>
    <title>untitled</title>
</head>
<body>
    <script type="text/javascript" charset="utf-8">
        (function() {
            var addEvent = function(element, type, method) {
                if('addEventListener' in element) {
                    element.addEventListener(type, method, false);
                } else if('attachEvent' in element) {
                    element.attachEvent('on' + type, method);

                    // If addEventListener and attachEvent are both unavailable,
                    // use inline events. This should never happen.
                } else if('on' + type in element) {
                    // If a previous inline event exists, preserve it. This isn't
                    // tested, it may eat your baby
                    var oldMethod = element['on' + type],
                    newMethod = function(e) {
                        oldMethod(e);
                        newMethod(e);
                    };
                } else {
                    element['on' + type] = method;
                }
            };

            // Work around IE 6/7 bug where form submission targets
            // a new window instead of the iframe. SO suggestion here:
            // http://bit.ly/6CzWIF
            var iframe;
            try {
                iframe = document.createElement('<iframe name="postHere">');
            } catch (e) {
                iframe = document.createElement('iframe');
                iframe.name = 'postHere';
            }

            iframe.name = 'postHere';
            iframe.id = 'postHere';
            iframe.src = '/emptypage.php';
            addEvent(iframe, 'load', function() {
                alert('iframe load');
            });

            document.body.appendChild(iframe);

            var form = document.createElement('form');
            form.target = 'postHere';
            form.action = '/emptypage.php';
            var submit = document.createElement('input');
            submit.type = 'submit';
            submit.value = 'Submit';

            form.appendChild(submit);

            document.body.appendChild(form);
        })();
    </script>
</body>
</html>

The alert fires every time I click the submit button in Safari, Firefox, IE 6, 7 and 8.

eyelidlessness
Or provide some sort of accessor. I agree.
Upper Stage
It's helpful when people vote stuff down for them to explain their reasoning for voting down.
eyelidlessness
I did not down-vote. However, saying window['varName'] = varName is the same as making a global var declaration outside of the closure. `var foo = "bar"; (function() { alert(window['foo']) })();`
Josh Stodola
You answered the title, not the question. I didn't like that. Putting the closure idiom within the context of references from the inline event handler (as the meat of the question is getting at) would have been nicer.
Crescent Fresh
Josh Stodola, correct. That is because your inline event handler is calling a global function. With the closure, it is up to you to declare which variables (or functions) are global and which are not. It gives you full control over what (if anything) to make global.
eyelidlessness
Crescent Fresh, I answered the question. The question's assumptions would need to be redesigned in order to avoid global functions. This answer takes, as a guideline, the question's assumptions (an inline event handler, for instance) and allows the developer to choose only the necessary global access points rather than everything being in the global scope.
eyelidlessness
@eyelidlessness: Holy hell, good edit, +1. @Josh mentioned in a comment on another question: "The inline handler is required in this situation. When form submits to iframe, your onload handler set programmatically will not fire." Can someone confirm, `iframe.onload = ...` is not equivalent to `<iframe onload="..."`?
Crescent Fresh
F**kin hell, what is with this "Vote too old" BS??
Crescent Fresh
I hate the "vote too old" bs too. I spent quite a bit of time yesterday assigning onload to the ifframe from the code-behind, and it simply would not work after the initial load. As soon as it displayed the response from the first upload, that was it. I also found it impossible to get the form to work properly with injecting the Iframe into the DOM. It has to be hard-coded in the markup.
Josh Stodola
Hold on, I'll build a quick test case of the `load` event. I really don't think it should be behaving the way you describe.
eyelidlessness
I've done some testing now, and yes you can assign the load event programmatically using your "addEvent" function. And I know what I was doing wrong yesterday. It's embarassing, really. I was doing `$(frames["postMe"]).load()` so I was the frame object instead of the element!
Josh Stodola
I guess I was just a little too late with my test case! Oh well, it's there for future reference anyhow.
eyelidlessness
I feel like such an idiot. I know I tried a bare-bones getElementById("postMe").onload = function() { } yesterday too, but I supposed that failed because I forgot to remove the logic required to bypass the very initial page load (of the parent) which seemed to always fire the inline onload. Thanks for your help, I've learned a lot today!
Josh Stodola
There, I can finally upvote now, Cripes. @eyelidlessness: nice work.
Crescent Fresh
A: 

some things are going to be in the global namespace -- namely, whatever function you're calling from your inline javascript.

In general, the solution wrap everything in a closure:

(function() {
   var uploadCount = 0;
   function startupload() {  ...  }
   document.getElementById('postHere').onload = function() { 
       uploadCount ++;
       if (uploadCount > 1) startUpload();
   };
})();

and avoid the inline handler.

Jimmy
The inline handler is required in this situation. When form submits to iframe, your onload handler set programmatically will not fire.
Josh Stodola
@Josh: whoa, really? `iframe.onload = ...` is not equivalent to `<iframe onload="..."`?
Crescent Fresh
A: 

Can we agree to use the singleton pattern and hide the details of the implementation?

Upper Stage
http://stackoverflow.com/questions/726685/yeah-i-know-im-a-simpleton-so-whats-a-singleton/761399#761399
Ates Goral
I like the article - thanks. The author writes "...[all] variables/objects ... reachable by traversing the object graph... [are] global! Singletons, usually are complex objects which contain a lot of state. As a result all of the state of Singleton is global as well." I prefer these details be hidden: http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html I suspect it's easier to agree to avoid the Singleton pattern rather than discuss the merits of data hiding.
Upper Stage
+2  A: 

Sometimes it makes sense to have global variables in JavaScript. But don't leave them hanging directly off window like that.

Instead, create a single "namespace" object to contain your globals. For bonus points, put everything in there, including your methods.

Nosredna
+7  A: 

I suggest the module pattern.

YAHOO.myProject.myModule = function () {

    //"private" variables:
    var myPrivateVar = "I can be accessed only from within YAHOO.myProject.myModule.";

    //"private" method:
    var myPrivateMethod = function () {
     YAHOO.log("I can be accessed only from within YAHOO.myProject.myModule");
    }

    return  {
     myPublicProperty: "I'm accessible as YAHOO.myProject.myModule.myPublicProperty."
     myPublicMethod: function () {
      YAHOO.log("I'm accessible as YAHOO.myProject.myModule.myPublicMethod.");

      //Within myProject, I can access "private" vars and methods:
      YAHOO.log(myPrivateVar);
      YAHOO.log(myPrivateMethod());

      //The native scope of myPublicMethod is myProject; we can
      //access public members using "this":
      YAHOO.log(this.myPublicProperty);
     }
    };

}(); // the parens here cause the anonymous function to execute and return
erenon
I will +1 because I understand this and it is extremely helpful, but I am still unclear as to how effective this would be in the situation where I only use one global variable. Correct me if I am wrong, but executing this function and returning it causes the object returned to be stored in `YAHOO.myProject.myModule`, which is a global variable. Right?
Josh Stodola
@Josh: global variable is not evil. Global variable_S_ are evil. Keep the count of globals as few as possible.
erenon
+1  A: 
window.onload = function() {
  var frm = document.forms[0];
  frm.target = "postMe";
  frm.onsubmit = function() {
    frm.onsubmit = null;
    var uploader = new LazyFileUploader();
    uploader.startUpload();
    return false;
  }
}

function LazyFileUploader() {
    var uploadCount = 0;
    var total = 10;
    var prefix = "FileUpload"; 
    var upload = function() {
     var fil = document.getElementById(prefix + uploadCount);

     if(!fil || fil.value.length == 0) {    
      alert("Finished!");
      document.forms[0].reset();
      return;
      }

     disableAllFileInputs();
     fil.disabled = false;
     alert("Uploading file " + uploadCount);
     document.forms[0].submit();
     uploadCount++;

     if (uploadCount < total) {
      setTimeout(function() {
       upload();
      }, 100); 
     }
    }

    this.startUpload = function() {
     setTimeout(function() {
      upload();
     }, 100);  
    }   
}
ChaosPandion
How do I increment the uploadCount inside of my `onload` handler on the iframe? This is crucial.
Josh Stodola
OK, I see what you did here. Unfortunately this is not the same. This fires all uploads seperately, but at the same time (technically, there is 100ms in between them). The current solution uploads them sequentially, meaning the second upload does not start until the first upload is completed. That's why the inline `onload` handler is required. Programmatically assigning the handler does not work, because it only fires the first time. The inline handler fires every time (for whatever reason).
Josh Stodola
I am still going to +1 though, because I do see that this is an effective method of hiding a global variable
Josh Stodola
You could create an iframe for each upload to maintain the individual callbacks.
Justin Johnson
A: 

First off, it is impossible to avoid global JavaScript, something will always be dangling the global scope. Even if you create a namespace, which is still a good idea, that namespace will be global.

There are many approaches, however, to not abuse the global scope. Two of the simplest are to either use closure, or since you only have one variable you need to keep track of, just set it as a property of the function itself (which can then be treated as a static variable).

Closure

var startUpload = (funtion() {
  var uploadCount = 1;  // <----
  return function() {
    var fil = document.getElementById("FileUpload" + uploadCount++);  // <----

    if(!fil || fil.value.length == 0) {    
      alert("Finished!");
      document.forms[0].reset();
      uploadCount = 1; // <----
      return;
    }

    disableAllFileInputs();
    fil.disabled = false;
    alert("Uploading file " + uploadCount);
    document.forms[0].submit();
  };
})();

* Note that incrementing of uploadCount is happening internally here

Function Property

var startUpload = function() {
  startUpload.uploadCount = startUpload.count || 1; // <----
  var fil = document.getElementById("FileUpload" + startUpload.count++);

  if(!fil || fil.value.length == 0) {    
    alert("Finished!");
    document.forms[0].reset();
    startUpload.count = 1; // <----
    return;
  }

  disableAllFileInputs();
  fil.disabled = false;
  alert("Uploading file " + startUpload.count);
  document.forms[0].submit();
};

I'm not sure why uploadCount++; if(uploadCount > 1) ... is necessary, as it looks like the condition will always be true. But if you do need global access to the variable, then the function property method I described above will allow you to do so without the variable actually being global.

<iframe src="test.htm" name="postHere" id="postHere"
  onload="startUpload.count++; if (startUpload.count > 1) startUpload();"></iframe>

However, if that's the case, then you should probably use an object literal or instantiated object and go about this in the normal OO way (where you can use the module pattern if it strikes your fancy).

Justin Johnson
A: 

Use closures. Something like this gives you a scope other than global.

(function() {
    // Your code here
    var var1;
    function f1() {
        if(var1){...}
    }

    window.var_name = something; //<- if you have to have global var
    window.glob_func = function(){...} //<- ...or global function
})();
NilColor