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

allow accessing dependant attributes which have a reserve keyword as name #22

Conversation

GustavoCaso
Copy link
Member

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

factories.define(:announcement) do |f|
  f.when { ROM::SQL::Postgres::Values::Range.new(3, 9, :'[]') }
  f.begin { |when_| when_.lower }
  f.end { |when_| when_.upper }
end

@@ -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)]
Copy link
Member

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

Copy link
Member Author

@GustavoCaso GustavoCaso Mar 24, 2018

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

@GustavoCaso GustavoCaso force-pushed the allow-to-use-reserve-keywords-for-dependant-attributes branch from 8634a4f to 317cf70 Compare March 24, 2018 11:42
@solnic
Copy link
Member

solnic commented Mar 26, 2018

Maybe we should switch to kwargs for this feature?

@flash-gordon
Copy link
Member

@solnic I thought about it, it's not gonna work, you still need to access local variable with reserved words

proc { |when:| "how to access >when< here?" }

@timriley
Copy link
Member

timriley commented Mar 26, 2018

I guess binding.local_variable_get is a bit too much to ask of end users.

@timriley
Copy link
Member

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)}" }

@flash-gordon
Copy link
Member

@timriley ** not gonna work either, rom-factory uses deps for figuring out the order in which callable values have to be called (via tsort).

@timriley
Copy link
Member

@flash-gordon Ah, of course. Well in this case a trailing underscore sounds like the least worse solution 😅

@GustavoCaso
Copy link
Member Author

@solnic @flash-gordon @timriley I know is not ideal but shall we push it?

@solnic
Copy link
Member

solnic commented Mar 26, 2018

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 }

@flash-gordon
Copy link
Member

@solnic this can be slower because may require a wrapper but I'm not sure

@solnic
Copy link
Member

solnic commented Mar 26, 2018

@flash-gordon yeah but it won't require tsorting

@flash-gordon
Copy link
Member

@solnic sorting is required just once whereas wrapping is required on every instantiation, no?

@solnic
Copy link
Member

solnic commented Mar 26, 2018

@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

@GustavoCaso
Copy link
Member Author

@solnic @flash-gordon I can have a look and try to implement it, or is anyone interested in do it?

@flash-gordon
Copy link
Member

@GustavoCaso depends on whether @solnic's thinking process complete :)

@solnic
Copy link
Member

solnic commented Mar 27, 2018

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 f.foo { |bar:| } would make bar available via wrapper object, which I think would be cool.

So, Gustavo, if you're interested in experimenting with this, I'd say go ahead :)

@flash-gordon
Copy link
Member

|bar:| won't work, only |bar:, **| will

@solnic
Copy link
Member

solnic commented Mar 27, 2018

@flash-gordon gahhh right ;( but it still makes sense to experiment with this

@solnic
Copy link
Member

solnic commented Jul 27, 2020

Closing due to lack of activity. The related issue remains open, so we should revisit this before 1.0.0.

@solnic solnic closed this Jul 27, 2020
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

Successfully merging this pull request may close these issues.

Access other attributes named like reserverd words in ruby (new, end, when)
4 participants