4
\$\begingroup\$

I know using global variables in JavaScript is bad practice. Here I am using config, host and port as global variables as I have added a watch function to watch for the changes in the config.json file. Is the use of global variables in this example a bad practice or is it ok to use them?

config.json file

{
    "host":"127.0.0.1",
    "port":8000
}

server.js file

(function(){

    var fs = require("fs");
    config = JSON.parse(fs.readFileSync("config.json")); //global variable
    host = config.host;  //global variable
    port = config.port;  //global variable
})();


var http = require("http");
var server = http.createServer(function(request,response){
    response.writeHead(200,{"Content-Type":"text/plain"});
    response.end("Hello World");
});

server.listen(port,host,function(){
    console.log("Server listening at:"+host+" on port:"+port);
});


fs.watchFile("config.json",function(){
    config = JSON.parse(fs.readFileSync("config.json"));  //global variable(overwritten)
    host = config.host;  //global variable(overwritten)
    port = config.port;  //global variable(overwritten)

    server.close();
    server.listen(port,host,function(){
          console.log("Server listening at:"+host+" on port:"+port);
       };
    }
});
\$\endgroup\$
1
  • 1
    \$\begingroup\$ You don't really need globals to accomplish this. Globals should be reserved to variables you want to expose to the environment. \$\endgroup\$
    – megawac
    May 25, 2014 at 17:13

2 Answers 2

2
\$\begingroup\$

The "global" you mean here is the module scope. In NodeJS, each file has it's own scope, unlike browser JS where every file runs in a single global scope. This code is too small to worry about globals, and you'll be splitting your code into several files/modules anyways to keep it maintainable. TL;DR - don't worry about globals.

As for your code, here's a few tweaks. My NodeJS is a bit rusty, but I think this works:

// Declare dependencies up top. Makes them easier to see.
var fs = require("fs");
var http = require("http");
var configFile = 'config.json';

function readConfig(file){
  return JSON.parse(fs.readFileSync(file));
}

function runServerWithConfig(server,configFile){
  // close server if it exists
  if(server) server.close();

  // read config
  var config = readConfig(configfile);

  // listen using new config
  server.listen(config.port,config.host,function(){
    console.log("Server listening at:"+config.host+" on port:"+config.port);
  });
}


var server = http.createServer(function(request,response){
    response.writeHead(200,{"Content-Type":"text/plain"});
    response.end("Hello World");
});

// Run and watch
runServerWithConfig(server,configFile);
fs.watchFile(configFile,function(){
  runServerWithConfig(server,configFile);
});
\$\endgroup\$
2
\$\begingroup\$

You say you're using globals because "I have added a watch function to watch for the changes in the config.json file". You don't need globals for this. If you declare the variables at the top level, like you've done with http and server, they'll be accessible everywhere in your script, including inside the anonymous function where you're using them.* You could get rid of the (function() { ... })(); wrapper round the first bit (not sure why you've wrapped that bit) and then declare them there.

"I know using global variables in JavaScript is bad practice." Using global variables in any language is bad practice if you can avoid it.

*A function like this, which uses variables declared outside the function, is called a "closure".

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.