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

Using as everywhere in the proxy chain #914

Open
cheerfulstoic opened this issue Aug 14, 2015 · 3 comments
Open

Using as everywhere in the proxy chain #914

cheerfulstoic opened this issue Aug 14, 2015 · 3 comments

Comments

@cheerfulstoic
Copy link
Contributor

This is an idea from @leviwilson

Right now you can just use as at the start of a query chain for models (like Model.as(:foo))

What if we allowed for as anywhere in the chain to define node/relationship variables? Also, what if we took away the arguments for association methods to define those same variables?

I realize that we'd need to do a lot of refactoring. Also users of the gem would need to as well, so we'd want to deprecate the association variables. But I think it would make our code simpler and I think it would make the syntax make a lot more sense.

  • There wouldn't be confusion about when to use as
  • optional(:association) would be cleaner and I think make more sense
  • You could re-define variables when you've been passed a proxy object (I haven't thought this one all the way through)

Would be good to get lots of feedback on this item from lots of people. Of course feedback from @subvertallchris would be great (I know you're busy, so no rush. I don't see this happening anytime soon ;)

@cheerfulstoic
Copy link
Contributor Author

Also I know @jexp was confused by the syntax, so I'd welcome his feedback as well

@subvertallchris
Copy link
Contributor

This would be really nice as an option. Things like this would be simpler:

def my_method
  some_object.scoped_query.as(:q)
end

private

def self.scoped_query
  all.my_association.where(value: false)
end

At the moment, you have to either set the values in your scoped method or give them as arguments. It can be confusing and it's so unintuitive.

I worry about the amount of refactoring it would force for users relative to the benefit offered. Would it be impossible to leave the old version as an option without firing deprecation notices?

@cheerfulstoic
Copy link
Contributor Author

Yeah, certainly we could add the option without the deprecation warnings. A couple of things to consider, though:

  • I think the deprecations (If we use ActiveSupport's Deprecation module) only fire in development and test environments.
  • The deprecation (if implemented correctly) would tell people the lines where it's happening
  • People can fix it by just adding the three characters .as in between the association and it's arguments

We could also add a configuration variable so that they could disable the warning if we wanted to.

I think it would simplify our code a lot, though.

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

No branches or pull requests

2 participants