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

Suggestion: Unset keys should be falsy #98

Open
DannyBen opened this issue Mar 16, 2018 · 3 comments
Open

Suggestion: Unset keys should be falsy #98

DannyBen opened this issue Mar 16, 2018 · 3 comments

Comments

@DannyBen
Copy link

DannyBen commented Mar 16, 2018

I love configatron, but although it solves a lot of things, it makes it rather difficult to work with unassigned keys.

In a standard hash, or an OpenStruct, non existing values are falsy:

o = OpenStruct.new({ one: '1', two: '2' })
p o.three
# => nil

While in configatron they are truthy:

# configatron.allow = false   # unset
p configatron.allow ? "allowed" : "not allowed"
# => "allowed" (but expected "not allowed")

Even if I want to set defaults, I cannot work with the common ruby syntax:

configatron.allow ||= false

And instead, I can only do:

configatron.allow = false unless configatron.has_key? :allow

And in cases where I can't (or prefer to avoid) having defaults, in order to allow "open schema", the only way for me to handle unset keys in the same way as falsy keys, is:

allowed = configatron.has_key?(:allow) ? configatron.allow : false
p allowed ? "allowed" : "not allowed"

Am I missing some functionality in configatron that will make this easier?

If there is no way to alleviate the above issues, I would love to see a change that allows it - even if it will require me to opt-in to this behavior using some config directive (in case it is desired to avoid introducing a breaking change).

@ghost
Copy link

ghost commented Mar 16, 2018

After looking at the code a little I found a way to do what you want.

[14] pry(main)> configatron.fetch(:helloworld, true)
=> true
[15] pry(main)> configatron.fetch(:helloworld)
=> true
[16] pry(main)> configatron.fetch(:foobarbaz)
=> nil

The [] method of the Store class seems to be a mere wrapper around fetch.

I think you shouldn't use the fetch method because then your request would lack an exit door when the store is locked. (See store.rb#L41.) Shouldn't this step be moved in the fetch function ? Then it should be possible to use it to pass a default value ?

@ghost
Copy link

ghost commented Mar 16, 2018

I have made a little something by merging [] and fetch together that behaves like the following:

[3] pry(main)> configatron.allow
=> nil

I'm opening a pull request so we can discuss that.

@ghost ghost mentioned this issue Mar 16, 2018
@ghost
Copy link

ghost commented Mar 16, 2018

So I was clearly mistaken - sry guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant