Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow specification of default properties in code #110

Closed
benmccann opened this issue Nov 22, 2013 · 5 comments
Closed

Allow specification of default properties in code #110

benmccann opened this issue Nov 22, 2013 · 5 comments

Comments

@benmccann
Copy link
Contributor

Play wraps the Typesafe config in a scala object in another Java object. It might make sense for Play to expose the Typesafe config to end users. The largest reason this hasn't been done is that the Play config allows specification of default properties in code while the Typesafe config does not. See a related bug in Play for some initial discussion on the topic: playframework/playframework#2059

@havocp
Copy link
Collaborator

havocp commented Nov 22, 2013

I added discussion of the status quo to the README, see 41f3de8

basically, play returns null. So you write (in Java) something like:

Boolean b = conf.getBoolean("foo");
if (b == null)
  b = true;

With the config library now, you can write:

boolean b = true;
try { b = conf.getBoolean("foo"); } catch (ConfigException.Missing e) {}  // throws and leaves b alone

or:

boolean b;
if (conf.hasPath("foo"))
  b = conf.getBoolean("foo");
else
  b = true;

I'm not sure any of those are all that bad, but they do NOT introduce the risk of NullPointerException (if something is unexpectedly unset and you don't check for it, you'll get a useful error message, not NPE).
If you really want null, you can also use the ConfigObject API.

The reason for the default Config behavior is that it's much safer in the common case, instead of making the common case dangerous to optimize for a rare case.

There are also a variety of ways to ensure that there will be a default, as discussed in that README patch (reference.conf, or programmatically merge defaults into your Config), and those ways put the default in ONE spot instead of at each getter. So again, we avoid a pitfall (cut-and-pasting defaults).

NPE and cut-and-paste defaults are the two dangers we're trying to avoid, and also we're trying to make the 95% case what the API optimizes for.

But even with that said, I don't think the null check you need with the Play API is any easier to use than the other approaches I've mentioned here.

@havocp
Copy link
Collaborator

havocp commented Nov 22, 2013

I guess the other likely API suggestion here would be getBoolean("foo", true) which does avoid the NPE risk but keeps the cut and paste of default to every spot that gets the setting. This was what I expected Play would have in fact but looking at the API docs it seems to just have null-returning getters.

People do sort of look for this API since some other APIs have it, but I hate to add something people will tend to use out of habit when the lib offers literally 4 or 5 safer ways to do it.

@benmccann
Copy link
Contributor Author

I'm fine if you want to close this bug. I was curious if there was a way we could unify they APIs, but it seems like it might be a bit tricky. I appreciate the explanation and the documentation.

@havocp
Copy link
Collaborator

havocp commented Nov 25, 2013

I'm fine leaving it open to discussion or hearing other ideas here. Just wanted to state my current perspective on it. I think it is a good goal to avoid the wrapper in Play.

@havocp
Copy link
Collaborator

havocp commented Mar 7, 2015

This is the same as #186 I guess so will close this copy.

@havocp havocp closed this as completed Mar 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants