Enforcing set environment variables

Context

I have a node.js application that uses many other modules and libraries. Some of these modules pull details from process environment variables, such as URL's for databases, credentials, and so on. In particular, because this node app is running in Bluemix, there's a large JSON variable called VCAP_SERVICES that contains all the connection information the app may need (users/passwords, names, conditions, and target URL's).

When the application is running in the "real" Bluemix environment, it will have these variables set. But, when the application is running in development, or debug mode it is convenient to set these variables in a more friendly way other than specifying multiple complicated, large variables.

To solve this problem, I have created a config.js script which looks in a specific folder for files with a specific extension .env. If a file is found with that extension, the environment variable with that file's name is inspected. If the variable is set, it is left alone. If the variable is not set, it is set to have the value of the file's contents.

This has to happen synchronously because this work has to complete before other modules have loaded.

To help things along, and to simplify the code management process, I have put this all in to a sub-folder, and created a .gitignore file that ignores all files with the extension .env. This prevents any passwords or other tokens from being committed to the source code repository.

Here's an example context of how the code would be used:

```
/jshint node:true/

//------------------------------------------------------------------------------
// node.js starter application for Bluemix
//------------------------------------------------------------------------------

// ensure all environment variables are set appropriately
require('./config/config.js');

// This application uses express as it's web server
// for more info, see: http://expressjs.com
var express = require('express');
var logger = re

One simple thing I see as a (small performance) improvement would be to store the length of your set of files.

You have this tiny for:

for (var i = 0; i < cfgfiles.length; i++) {

Try to replace it with:

for (var i = 0, length = cfgfiles.length; i < length; i++) {

This speeds up the code because you don't have to access to the length property in the object. Accessing to properties in objects is always slower than accessing to local variables.

A few lines below, you have this:

if (process.env[name]) {
    //console.log("Environment variable " + name + " already set: ignoring");
} else {
    var val = fs.readFileSync(file, 'utf8');
    process.env[name] = val;
    console.log("Loaded unset environment variable " + name + " from file " + file);
    //console.log("setting environment " + name + " to config file " + file + ":\n   " + val);
}

Besides the comments being there, which are useless to those who try to read the code, you have a completely useless if:

if (process.env[name]) {
    //console.log("Environment variable " + name + " already set: ignoring");
}

Which does absolutely nothing.

You could use it to set a new value or remove it entirely.

Also, the way you check the value is incorrect. What if the value is 0? It will be re-written! What if it is ''? It won't! There you go, you have a bug/inconsistent behaviour.

Rewrite it to something like this:

if (!(name in process.env)) {
    var val = fs.readFileSync(file, 'utf8');
    process.env[name] = val;
    console.log("Loaded unset environment variable " + name + " from file " + file);
}

As an alternative, you can use this if:

if (!process.env.hasOwnProperty(name)) {

The difference is that this method will check if a determined property exists within the object's own properties (the ones you provided). Inherited properties from the Object prototype would return false here. This is only a concern if you have a file called toString or constructor or hasOwnProperty. Depending on what you are trying to do, you may not care about those methods (except the last one).

Also, you have a lack of comments here:

name = name.replace(/\.env$/, "");
file = path.join(__dirname , file);

So, what are you trying to do? It took me 10 minutes to understand what was going on there. A comment would have been nice.

Also, as a small nitpicky point, you have the regexp /\.env$/ being repeated. You should store it in a variable. Also, consider adding the i flag, so that abc.ENV can still be matched.

As a final note, and not knowing how node.js works in this regard, you could try to wrap your code in an anonymous function. This avoids leaking variables everywhere once you are done. But I don't know how it would work or not. If it indeed worked. For 'regular' Javascript, I use the following:

(function(window, undefined) {
    [code]
})(Function('return this')());

Which isn't the most readable thing, but works as intended.

StackExchange Code Review Q#97826, answer score: 7