Refactor Chain::DSL for use with instances of itself #10
Conversation
* Use #module_eval instead of #class_eval
| @registry.each_with_object(Class.new(DSL)) { |(name, builder), dsl| | ||
| define_dsl_method(name, builder, dsl) | ||
| } | ||
| def compile_dsl_module |
There was a problem hiding this comment.
Is it necessary to have a public visibility for this? In general, by default, I think of all methods as private unless they are absolutely necessary in the public interface to allow things to work.
In this case the method is kind of a command and query method. It does some work and returns the @dsl_module. You can't call it without that extra work being performed.
What I might suggest is either having an #initialize_dsl_module method that sets up the state of the object, or have a #dsl_module method that initializes the module and does all the work adding the dsl methods one time and memorizes it in @dsl_module.
There was a problem hiding this comment.
@dkubb thx for catching that! there's no need to expose that method to the public.
Currently I can't imagine ever using #use directly myself. Maybe once someone makes strong cases for that, we can reconsider.
* Moved and refactored Chain::DSL::Builder into Chain::DSL::Config::Builder
Bundling mutant on the previously failing rubies has been fixed lately.
It always bugged me that
Environmentneeded to get theregistryandchain_dslinjected separately when it feels likechain_dslshould know the registry. Also, it kinda bugged me that a newChain::DSLsubclass was created for every environment.The real reason tho, is that with moving
registryaccess inside achain_dslinstance,Environment#initializecan be changed to only acceptchain_dsl(for now). This will "open up 2 more slots" for parameters to#initialize- i like 3 params at most). In the future, I will then inject a mutable action registry and the application environment intoEnvironment#initialize, in addition tochain_dsl.This will eventually allow for decentralized dispatch table definition, by providing something like
Environment#chain(name, other = Chain::EMPTY, &block)wherenamewill be used to register the created chain with the mutable registry. Furthermore,Environment#dispatcherwill need no more parameters, as the populated dispatch table (the action registry) as well as the application environment, will already be available inside anEnvironmentinstance. It is safe to assume that the (user provided) application environment object is already loaded at the timeSubstation::Environmentgets instantiated. This is because the app environment is part of the app, which by itself, should know nothing about substation anyway.This obviously needs more work, but integration specs pass already. Once unit specs pass too, I will merge this into the
decentralized_dispatch_tablebranch and continue working from there.