Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Improved app.set to handle object parameter #1333

wants to merge 2 commits into


None yet
4 participants

yhpark commented Sep 16, 2012

Many app.sets look a bit ugly.
It can be cleaner if app.set handles object parameter.

For example,

app.set("db_one", "");
app.set("db_two", "");
app.set("db_three", "");
app.set("db_four", "");

above can be rewritten as below

  db_one: "",
  db_two: "",
  db_three: "",
  db_four: "",

In addition, this also enables below use case.

// conf.js
module.exports = {
  db_one: "",
  db_two: "",
  db_three: "",
  db_four: "",
// during app configuration

yhpark added some commits Sep 16, 2012

app.set with object parameter
improved app.set to handle object parameter.

tj commented Sep 16, 2012

-1 from me, in 4x this api will only remain for internal settings, JSON conf works better in general for an app anyway, and personally I think objects look a lot worse than multiple calls

rummik commented Sep 16, 2012

@visionmedia But objects are pretty much JSON?

Anyway, I'd have to agree that JSON would be better for configuration, but this system does have the advantage of being a quick way for everything to be configured within an app. (And with this commit, or an equivalent, it would be a small step away from accepting JSON)


tj commented Sep 16, 2012

I dont want to make any assumptions about how people should configure their apps, later I'll ditch .configure() but that's not until 4x. Things like nconf / eson are better solutions but still nothing that should be (or benefits from being) baked into the framework


jonathanong commented Sep 9, 2013

closing because tj -1'd it

@jonathanong jonathanong closed this Sep 9, 2013

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