Skip to content

Instantly share code, notes, and snippets.

@tj
Last active December 27, 2015 14:49
Show Gist options
  • Save tj/7343489 to your computer and use it in GitHub Desktop.
Save tj/7343489 to your computer and use it in GitHub Desktop.
/**
* Module dependencies.
*/
var pkg = require('../package');
var env = process.env.NODE_ENV || 'development';
/**
* Return setting `name`.
*
* @param {String} name
* @return {Mixed}
* @api public
*/
module.exports = function(name){
if (null == pkg[env]) throw new Error('invalid configuration environment "' + env + '"');
if (null == pkg[env][name]) throw new Error('invalid configuration key "' + name + '"');
return pkg[env][name];
};
@inadarei
Copy link

inadarei commented Nov 6, 2013

Throwing an exception for a missing configuration key sounds ...err... somewhat rude? :)

Also, even in the simplest of cases you would want the configuration defaulting feature, so maybe:

var pkg         = require('../package');
var env         = process.env.NODE_ENV || 'development';
var DEFAULT_ENV = 'default';

module.exports = function(name){
  if (null == pkg[env]) throw new Error('invalid configuration environment "' + env + '"');
  if (null == pkg[env][name]) {
    if (null != pkg[DEFAULT_ENV][name]) {
      return pkg[DEFAULT_ENV][name];
    }
    return null;
  }
  return pkg[env][name];
};

@esco
Copy link

esco commented Nov 6, 2013

@inadarel
If your missing a config key for an api or database value then your application probably won't work. It seems like not having that would make the application state useless.

Why have a DEFAULT_ENV value? Thats the purpose of the string after || operator. It works out the same way.

@inadarei
Copy link

inadarei commented Nov 7, 2013

@menzoic, the question of fatality for a missing setting is generally up to the application code to decide, though, and it's shorter/prettier to write:

var configvar = config('configvarname') || 'default_value'; 

than the whole try.. catch.. thing. Epsecially when you'll be doing it all over the code, for every setting. Throwing exceptions in Node is a risky business. While this function is not async, its caller probably will be, so it's not like you can trust traditional bubbling-up of the exceptions along the call stack. Side effects can be ugly.

Regarding your second question, code on line 6 in the original does something different, it says: "if no environment is defined, assume 'development'", which is not the same as what DEFAULT_ENV does.

The need for DEFAULT_ENV stems from the fact that typically settings for various environments are mostly the same, and you only customize some settings for each environment. You wouldn't want to duplicate the common settings in each env config. maybe COMMON_ENV is a more clear, better name, but in my experience, most people call it default settings.

@tj
Copy link
Author

tj commented Nov 7, 2013

assume production would be safer as far as precedence goes, I've found throwing extremely helpful, little typos in config etc go undetected, it would have to be a pretty massive fail to have that throw in production, so unless you don't write tests or use your app at all it's fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment