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

ordering of option declarations should not prohibit forward reference in default procs #97

Open
ahoward opened this issue Jun 30, 2022 · 6 comments

Comments

@ahoward
Copy link

ahoward commented Jun 30, 2022

Describe the bug

the :default keyword supports an "instance_exec" type of evaluation whereby default: -> { foo + bar }' reference the object in questions' fooand barmethods. however, when eitherfooorbarare, themselves, dry-initialized attributes, the default value will resolve tonil` due to being referenced prior to 'dry initialization'

To Reproduce

require 'dry-initializer'

class A
  extend Dry::Initializer
  option :foo, default: -> { bar }
  option :bar, default: -> { 42 }
end.new

class B
  extend Dry::Initializer
  option :bar, default: -> { 42 }
  option :foo, default: -> { bar }
end.new

pp A.new #=> #<A:0x0000000103009958 @bar=42, @foo=nil>
pp B.new #=> #<B:0x000000010302aea0 @bar=42, @foo=42>

Expected behavior

@bar == @foo == 42

My environment

n/a

@flash-gordon
Copy link
Member

How would it work, though? Implementation-wise I mean.

@ahoward
Copy link
Author

ahoward commented Jun 30, 2022

essentially, the reader must be read consistent. specifically, the first invocation needs to memo-ize the default proc, vs simply looking at the list of options. eg.

ivar = "@#{ name }"

defined_method(name) do
  initialize_default_value_for(name) unless instance_variable_defined?(ivar)
  instance_variable_get(ivar)
end

@ahoward
Copy link
Author

ahoward commented Jun 30, 2022

i'll see if i can gin up a PR

@flash-gordon
Copy link
Member

But this would break the assumption an object is fully initialized after .new call. Normally, I don't mutate objects meaning I call .freeze after initialization. Your suggestion will break in such a use case.

@ahoward
Copy link
Author

ahoward commented Jul 6, 2022

not necessarily. one would simple need to hit all readers to initialize the values before freezing. freezing is nice but it also means that objects are expensive, as memoization is good for speed. regardless, it's a bug-ette in dry.rb now, since a specified default is not applied, depending on ordering. if this is by design so be it but i'm guessing it's a side-effect of other design goals. i think all goals can be achieve. the code is fairly abstract but i'm working on something now....

@flash-gordon
Copy link
Member

Freezing is not about the cost actually. It's a technique of separating boot time from runtime. An application stack and its constituents are meant to be immutable, hence frozen.

Anyway, handling such cases is somewhat problematic:

  option :foo, default: -> { bar }
  option :bar, default: -> { foo }

@nepalez nepalez removed the bug label Jul 26, 2022
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

3 participants