-
-
Notifications
You must be signed in to change notification settings - Fork 43
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 accessing dependant attributes which have a reserve keyword as name #22
allow accessing dependant attributes which have a reserve keyword as name #22
Conversation
lib/rom/factory/tuple_evaluator.rb
Outdated
@@ -80,7 +80,10 @@ def evaluate(*traits, **attrs) | |||
# @api private | |||
def evaluate_values(attrs) | |||
attributes.values.tsort.each_with_object({}) do |attr, h| | |||
deps = attr.dependency_names.map { |k| h[k] }.compact | |||
deps = attr.dependency_names.map do |key| | |||
h[remove_underscore(key)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is super minor but I wouldn't reveal the details in the name of the method, that is something like dependency_name
suits better. Next time you change the rules you'll need to change only the body of the method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally agree with it @flash-gordon 👍. done 317cf70
8634a4f
to
317cf70
Compare
Maybe we should switch to kwargs for this feature? |
@solnic I thought about it, it's not gonna work, you still need to access local variable with reserved words
|
I guess |
Another option if using kwargs would be to splat them and pull it off the hash: proc { |**args| "we can get it as #{args.fetch(:when)}" } |
@timriley |
@flash-gordon Ah, of course. Well in this case a trailing underscore sounds like the least worse solution 😅 |
@solnic @flash-gordon @timriley I know is not ideal but shall we push it? |
OK I need to think about this. Maybe this should be based on lazy evaluation, ie pass an object and whenever you call a method on it, it will generate a value based on factory definition. ie f.email { fake(:internet, :email) }
f.login { |s| s.email } |
@solnic this can be slower because may require a wrapper but I'm not sure |
@flash-gordon yeah but it won't require tsorting |
@solnic sorting is required just once whereas wrapping is required on every instantiation, no? |
@flash-gordon no, we need to create a wrapper for each factory object, so once per factory definition, then we just pass this one to each block when required |
@solnic @flash-gordon I can have a look and try to implement it, or is anyone interested in do it? |
@GustavoCaso depends on whether @solnic's thinking process complete :) |
Yeah I'd at least try to do it using a wrapper with injected factory, so that it can delegate to it in order to return a value from a factory. If we make it coercible to a hash, then as a bonus something like So, Gustavo, if you're interested in experimenting with this, I'd say go ahead :) |
|
@flash-gordon gahhh right ;( but it still makes sense to experiment with this |
Closing due to lack of activity. The related issue remains open, so we should revisit this before 1.0.0. |
Fixes #16
@solnic I was talking with @flash-gordon and we agree that this could be a good solution for this issue.
if you have a reserved keyword as one of your attributes
end
,do
,when
etc... you can append an underscore to the block argument and access inside the block.Example