Make adapters config work more natively #12
Conversation
dsalahutdinov
force-pushed
the
master
branch
from
2a21515
to
81742aa
Compare
|
Supposing profit of this PR is:
|
|
Having
That makes sense. Maybe, it's better to make
It doesn't matter in our case) |
|
Made some changes about wrapping the hash, does it now look better? |
lib/isolator/adapters/config.rb
Outdated
| class Config | ||
| include Enumerable | ||
| extend Forwardable | ||
| def_delegators :hash_config, :each, :[] |
There was a problem hiding this comment.
Let's use a shorter name, e.g. store
lib/isolator/adapters/config.rb
Outdated
|
|
||
| def register(id, adapter) | ||
| raise "Adapter already registered: #{id}" if respond_to?(id) | ||
| define_singleton_method(id) { self[id] } |
There was a problem hiding this comment.
Let's use the previous method_missing approach but without fallback to super (in order to raise NoMethodError when no such key).
The provided approach doesn't play well with deleting keys: after a key has been deleted we except to got an exception; and now it returns nil instead.
|
I've changed code for use |
| raise "Adapter already registered: #{id}" if Isolator.adapters.key?(id.to_s) | ||
| adapter = AdapterBuilder.call(**options) | ||
| Isolator.adapters[id.to_s] = adapter | ||
| AdapterBuilder.call(**options).tap do |adapter| |
There was a problem hiding this comment.
We should raise error before .call, 'cause we're patching a target module in .call.
|
Let's leave everything as is; no need for complexity |
|
👍 yes, it became too complicated. Thank you. |
A little bit clearly code of adapters config