one_to_one association naming

19 views
Skip to first unread message

John Firebaugh

unread,
Mar 10, 2010, 2:36:27 AM3/10/10
to seque...@googlegroups.com
The fact that one_to_one associations are a thin layer on one_to_many, specifically the need for them to be named in the plural, is causing a fair bit of accidental complexity in a custom plugin we have that does heavy reflection on associations.

What would be necessary to be able to name one_to_one associations in the singular?

Jeremy Evans

unread,
Mar 10, 2010, 2:16:29 PM3/10/10
to sequel-talk

I'm not sure of all the changes that would be needed to make it work.
It's possible that the changes are extensive enough to require the
creation of a real one_to_one association.

That being said, it would be helpful for you to explain what the
actual issues you are running into are. It's possible that we just
need to add something to the association reflection classes to
eliminate or at least reduce the complexity.

Jeremy

John Firebaugh

unread,
Mar 10, 2010, 3:05:41 PM3/10/10
to seque...@googlegroups.com
On Wed, Mar 10, 2010 at 11:16 AM, Jeremy Evans <jeremy...@gmail.com> wrote:
On Mar 9, 11:36 pm, John Firebaugh <john.fireba...@gmail.com> wrote:
> The fact that one_to_one associations are a thin layer on one_to_many,
> specifically the need for them to be named in the plural, is causing a fair
> bit of accidental complexity in a custom plugin we have that does heavy
> reflection on associations.
>
> What would be necessary to be able to name one_to_one associations in the
> singular?

I'm not sure of all the changes that would be needed to make it work.
It's possible that the changes are extensive enough to require the
creation of a real one_to_one association.

It was easy to get the specs to pass, but I'm not sure if I'm missing something. See the attached patch.
 
That being said, it would be helpful for you to explain what the
actual issues you are running into are.  It's possible that we just
need to add something to the association reflection classes to
eliminate or at least reduce the complexity.

We have a filter interface similar to iTunes smart playlists:


The filter criteria support many_to_one and one_to_one relationships using eager graphing. When serialized, criteria look something like this:

[['book.title', :=, "Where the Wild Things Are"],
 ['book.author.first_name', :=, "Maurice"],
 ['book.first_page.text', :contains, "Max"]]

Assume book -> pages is one-to-many, and book -> first page is one-to-one based on the constraint page.number = 1. The complexity arises because the accessor method is #first_page but the association name is :first_pages -- when processing the criteria, we have to special case 'book.first_page.text' to handle that. If we used 'book.first_pages.text' instead, we would still need special cases, just in different places. We really need the accessor name and association name to be the same.
0001-Support-singular-association-names-when-using-one_to.patch

Jeremy Evans

unread,
Mar 10, 2010, 6:16:45 PM3/10/10
to sequel-talk
On Mar 10, 12:05 pm, John Firebaugh <john.fireba...@gmail.com> wrote:

I'll say that I like the general idea of making one_to_one use a
singular, enough to break backwards compatibility. When we do this,
it would also be a good time to define a one_to_one method that uses
the :one_to_one option of one_to_many.

Your patch looks good. You should remove the singularize call, as
singularizing an already singular word can return a different word.

Have you had a chance to test eager loading with the patch?

class Book < Sequel::Model

one_to_many :first_page, :class=>:Page, :conditions=>{:page_number=>1}, :one_to_one=>true
end
Book.first.first_page
Book.filter(:id=>[1,2]).eager(:first_page)
Book.filter(:id=>[1,2]).eager_graph(:first_page)

I'm not sure if there are any eager loading specs that cover this
particular case, which weren't really necessary before as
the :one_to_one option just added a couple instance methods and didn't
modify eager loading behavior at all. I don't expect any breakage
from this patch, but it would be nice to test that area first.

Thanks for your help!

Jeremy

Reply all
Reply to author
Forward
0 new messages