Return and instance of Arel::Attributes when calling `[]` on the model. #264

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
Member

sikachu commented Apr 12, 2011

Hi,

I've discussed with Aaron a while ago about the way that I can use ARel's comparison methods (#gt, #lt, and so on) in AR. Turned out that I have to access it via #arel_table method of the model:

users = User.arel_table
famous_users = User.where(users[:fame].gt(80))

This become a bit tedious as I have to have another variable to store an instance of Arel::Table to be able to call those methods on it. Also, I don't think the code look clean if I merge those two lines together and make it as

famous_users = User.where(User.arel_table[:fame].gt(80))

So, after some discussion, I think it's better to make AR .[] method to return the instance of Arel::Attributes::*. This will give more readability to the code example above, and I think it fits in great with the context.

After applying my patch, you would then can do

famous_users = User.where(User[:fame].gt(80))

I think that looks really nice. I really wish I could get some feedback from you guys.

Return and instance of Arel::Attributes when calling `[]` on the model.
So, now you can do something like this:

  Topic.where(Topic[:ratings].gt(5))

Without having to call `#arel_table` first.

+1

Contributor

burke commented Apr 12, 2011

+so many. I use arel_table constantly. It feels clumsy having to type it everywhere. (re https://twitter.com/#!/tenderlove/status/57839406987493376)

Contributor

tadast commented Apr 12, 2011

+1

dombesz commented Apr 12, 2011

+1

@ghost

ghost commented Apr 12, 2011

+1

Agreed; This is a good idea. I use arel_table an awful lot in order to access predicates, and I find it rather clumsy to have to assign it first.

Contributor

oriolgual commented Apr 12, 2011

+1 idem as @scottlowe

Contributor

dolzenko commented Apr 12, 2011

+1 re https://twitter.com/#!/tenderlove/status/57839406987493376 (with other folks doing slighlty longer relation.table), using for things like .matches and .not_eq where there is some gain compared to using just bare SQL strings)

Contributor

txus commented Apr 12, 2011

Neat!

Contributor

sobrinho commented Apr 12, 2011

+1

-1, using the class twice does not sound good enough.

Contributor

mtodd commented Apr 12, 2011

+1

jneen commented Apr 12, 2011

Just to throw an idea out there. If it's a DSL, make it a DSL:

famous_awesome_users = User.where { fame.gt(100) & awesome? } # or .where { (fame > 80) & awesome? }

or is that too much magic? :)

Contributor

josepjaume commented Apr 12, 2011

+1 awesome!

Definitely +1. Makes not-writing SQL more attractive, which is a win for the universe.

Contributor

tobi commented Apr 12, 2011

You guys have no sense of what this kind of syntax would do to people new to Ruby. Fit's of despair would probably be a mild side effect. Don't forget, we are not trying to turn Ruby into a gulfing complete version of Perl, we are trying to create a framework which aids in Ruby's main mission of providing syntax that makes sense to people and just incidentally happens to be machine executable. .gt is broderline but something like fame.gt(100) & awesome? is just ridiculous.

in any case -1 from me.

famous_users = User.where(User[:fame].gt(80))

Real nice, but repeating User is just WRONG, unless I can do that:

famous_users = User.where(Posts[:age].gt(80))

grimen commented Apr 12, 2011

My opinion this DSL should be in core:

https://github.com/ernie/meta_where

So much more natural than the one suggested. Would never use the proposed syntax myself, just feels wrong/bloated.

tobi > I totally disagree. I just asked 3 people who have any idea in what is even ruby (not a jewel) : "what means for you fame.gt(100) & awesome, knowing that gt means greater than". They all answered me "fame superior to 100 and awesome ?". So I don't think it's a problem, I think the DSL idea makes sense.

And yeah. meta_where is just amazing.

Contributor

dpickett commented Apr 12, 2011

I'm inclined to agree with @grimen - the syntax just seems off and meta_where is something I frequently use.

I'm not looking forward to explaining the difference between user.attributes[:first_name] and User[:first_name] to newcomers.

I like the idea/concept, but I don't think the DSL is quite right yet.

Contributor

ernie commented Apr 12, 2011

Thanks for the MetaWhere love but I wouldn't advocate the existing MW go into core, and I wrote it.

Squeel (formerly MW 2.0) plays more nicely with others and the syntax is much improved, as well. It's still a work in progress but I hope to have it wrapped up by RailsConf.

Contributor

chanks commented Apr 13, 2011

Why not use the Mongoid-style syntax?

User.where(:fame.gt => 80, :awesome => true)

Edit: Never mind, I see that's what meta_where does too.

Contributor

ernie commented Apr 13, 2011

By way of example, Squeel DSL:

Article.joins{person}.
        where{title.matches('%hello%') & 
              person.name.matches('Ernie%')}.to_sql
=> SELECT "articles".* FROM "articles" 
   INNER JOIN "people" ON "people"."id" = "articles"."person_id"
   WHERE (("articles"."title" LIKE '%hello%' 
           AND "people"."name" LIKE 'Ernie%'))"

Person.joins{children.children.children}.
       where{children.children.children.name =~ 'Joe%'}.to_sql
=> SELECT "people".* FROM "people" 
   INNER JOIN "people" "children_people" 
     ON "children_people"."parent_id" = "people"."id" 
   INNER JOIN "people" "children_people_2" 
     ON "children_people_2"."parent_id" = "children_people"."id"
   INNER JOIN "people" "children_people_3" 
     ON "children_people_3"."parent_id" = "children_people_2"."id" 
   WHERE "children_people_3"."name" LIKE 'Joe%' 

Note.joins{notable(Person)}.
     where{(notable.id == 1) | (id + 3 == notable.id)}.to_sql
=> SELECT "notes".* FROM "notes" 
   INNER JOIN "people" ON "people"."id" = "notes"."notable_id" 
     AND "notes"."notable_type" = 'Person'
   WHERE (("people"."id" = 1 OR "notes"."id" + 3 = "people"."id"))

Person.select{name.op('||', '-diddly').as('flanderized_name')}.
       first.flanderized_name
=> "Aric Smith-diddly" 

Person.where{find_in_set(id, '1,2,3,4')}.to_sql
=> SELECT "people".* FROM "people"  WHERE (find_in_set("people"."id", '1,2,3,4'))

ZOMG, -Infinite then.

Owner

tenderlove commented Apr 13, 2011

@ernie has convinced me that this is probably not a good idea. See here. I think for now that if people want fancier syntax, they should use Squeel, so I'm closing this.

Thank you @sikachu for reopening this pull request so that we could get some good community feedback!

❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️

@tenderlove tenderlove closed this Apr 13, 2011

hisas pushed a commit to hisas/rails that referenced this pull request May 9, 2017

Merge pull request #264 from phildarnowsky/master
Shoulda-style RSpec matchers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment