CODEWITHSUNDEEP
javascriptperformance
user1584751
user1584751

Reputation:

JavaScript Best Practices - Global Variables

Can anyone provide feedback on how i can improve the script below? The script does work properly but uses Global Variables, I've been told that the use of global variables can cause issues in code.

    var vehicle = document.getElementById('vehicle');
    var residence = document.getElementById('residence');

    vehicle.setAttribute("class", "hide");

    document.getElementById('myList').addEventListener('change', function(){
        Array.prototype.forEach.call(document.querySelectorAll('.forms'), function(e){
            e.setAttribute("class", "hide");
        });

        var sel=+this.selectedIndex - 2;
        if(sel >= 0){
            vehicle.setAttribute("class", "show");
            residence.setAttribute("class", "hide");
        } else {
            residence.setAttribute("class", "show");
            vehicle.setAttribute("class", "hide");
        }
    });

Upvotes: 0

Views: 152

Answers (4)

sev7nx
sev7nx

Reputation: 21

For global variables you can use object "window":

window.vehicle = { param1 : "abc", param2 : document.getElementById('idXYZ').value};  // It is not necessary "var"

to access the variable :

window.vehicle.param1

Upvotes: 0

plalx
plalx

Reputation: 43718

It would be way better to modularize your code into testable and reusable components.

There are tons of ways to achieve this, but here's a simple example...

Define an object that represents your feature:

yourNS = window.yourNS || {};

yourNS.YourFeature = {
    init: function (options) {
        var vehiculeEl = document.querySelector(options.vehicleEl),
            residenceEl = document.querySelector(options.residenceEl),
            listEl = document.querySelector(options.listEl);

        vehiculeEl.className = 'hide';

        listEl.addEventListener('change', function () {

            var showVehicule = this.selectedIndex - 2 >= 0;

            vehiculeEl.className = showVehicule? 'show' : 'hide';
            residenceEl.className = showVehicule? 'hide' : 'show';
        });
    }
};

Use it:

!function () {
    yourNS.YourFeature.init({
        vehiculeEl: '#vehicule',
        residenceEl: '#residence',
        listEl: '#myList'
    });
}();

You might be interested by Writing Testable JavaScript, written by Rebecca Murphey.

Upvotes: 1

mxcd
mxcd

Reputation: 2404

Everyone has it's own JS style, but I prefer using a global JS object that stores all my variables but this only where it is really needed.

var GLOBAL = 
{
    key:"value",
    key2:variable,
    key3:
    {
        key4:variable,
        key5:'hans'
    }
};
GLOBAL.key3.key5 = 'peter';

An other option is to use a function that does everything, so the variables you use are contained in the namespace of your function.

var foo = function()
{
    var iAmInvisible = 42;
}

Upvotes: 0

dandavis
dandavis

Reputation: 16726

use a function for privacy:

<script type="text/javascript">
(function(){

    var vehicle = document.getElementById('vehicle');
    var residence = document.getElementById('residence');

    vehicle.setAttribute("class", "hide");

    document.getElementById('myList').addEventListener('change', function(){
        Array.prototype.forEach.call(document.querySelectorAll('.show'), function(e){
            e.setAttribute("class", "hide");
        });

        var sel=+this.selectedIndex - 2;
        if(sel >= 0){
            vehicle.setAttribute("class", "show");
        } else {
            residence.setAttribute("class", "show");
        }
    });

})();
</script>

doesn't get much easier: you don't need to alter your written code at all, just wrap it up.

Upvotes: 7

Related Questions