thr3ads.net - Rails - Improving this messy has_many :through with :include [Feb 2009]

If this information is useful, please help other people find it:
Share via:

Alan

2009-Feb-06 21:53 UTC

Improving this messy has_many :through with :include

I have a question about cleaning up a rather complex has_many. I''ll
describe the context in case it helps, but perhaps you can skip
straight to the code below.

* I am designing a fantasy football type system.
* I am modeling a Team which consists of many Players.
* I am connecting Teams and Players via a Transfer join model.
* A Transfer also has two GameWeeks (a GameWeek encapsulates the
start_time and end_time of a period of play).
* Transfer.from_end_of_gameweek indicates the GameWeek from which the
Player becomes part of the Team.
* Transfer.until_end_of_gameweek indicates the GameWeek from which a
Player ceases to be part of the Team.

So Transfers is essentially acting as a warehouse, storing history
about all player transfers that have taken place (in the future I
might make a dedicated warehouse but not yet).

Now, to determine which Players are playing for a Team at a given
instant I have this:

<pre>
class Team < ActiveRecord::Base
  has_many :transfers
  has_many :players, :through => :transfers,
    :include => [{ :transfers => :from_end_of_gameweek }, { :transfers
=> :until_end_of_gameweek }],
    :conditions => [''gameweeks.start_time <= ? AND
(gameweeks.end_time
IS NULL OR ? < gameweeks.end_time)'', Time.now.utc, Time.now.utc]
end
</pre>

Thus I can say team.transfers to get a list of all Transfers the Team
has made, or team.players to get the current squad.

It works, but that has_many is a pretty unpleasant bit of code and the
SQL doesn''t look the most efficient. The question is, can this be made
more elegant and maybe efficient?

Thanks.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

Alan

2009-Feb-07 11:51 UTC

head link

Improving this messy has_many :through with :include

I have a question about cleaning up a rather complex has_many. I''ll
describe the context in case it helps, but perhaps you can skip
straight to the code below.

* I am designing a fantasy football type system.
* I am modeling a Team which consists of many Players.
* I am connecting Teams and Players via a Transfer join model.
* A Transfer also has two GameWeeks (a GameWeek encapsulates the
start_time and end_time of a period of play).
* Transfer.from_end_of_gameweek indicates the GameWeek from which the
Player becomes part of the Team.
* Transfer.until_end_of_gameweek indicates the GameWeek from which a
Player ceases to be part of the Team.

So Transfers is essentially acting as a warehouse, storing history
about all player transfers that have taken place (in the future I
might make a dedicated warehouse but not yet).

Now, to determine which Players are playing for a Team at a given
instant I have this:

<pre>
class Team < ActiveRecord::Base
  has_many :transfers
  has_many :players, :through => :transfers,
    :include => [{ :transfers => :from_end_of_gameweek }, { :transfers
=> :until_end_of_gameweek }],
    :conditions => [''gameweeks.start_time <= ? AND
(gameweeks.end_time
IS NULL OR ? < gameweeks.end_time)'', Time.now.utc, Time.now.utc]
end
</pre>

Thus I can say team.transfers to get a list of all Transfers the Team
has made, or team.players to get the current squad.

It works, but that has_many is a pretty unpleasant bit of code and the
SQL doesn''t look the most efficient. The question is, can this be made
more elegant and maybe efficient?

Thanks.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

Frederick Cheung

2009-Feb-07 13:08 UTC

head link

Re: Improving this messy has_many :through with :include

On Feb 6, 9:53 pm, Alan
<alan.lar...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:> I have a question about cleaning up a rather complex has_many.
I''ll
> describe the context in case it helps, but perhaps you can skip
> straight to the code below.
>
> * I am designing a fantasy football type system.
> * I am modeling a Team which consists of many Players.
> * I am connecting Teams and Players via a Transfer join model.
> * A Transfer also has two GameWeeks (a GameWeek encapsulates the
> start_time and end_time of a period of play).
> * Transfer.from_end_of_gameweek indicates the GameWeek from which the
> Player becomes part of the Team.
> * Transfer.until_end_of_gameweek indicates the GameWeek from which a
> Player ceases to be part of the Team.
>
> So Transfers is essentially acting as a warehouse, storing history
> about all player transfers that have taken place (in the future I
> might make a dedicated warehouse but not yet).
>
> Now, to determine which Players are playing for a Team at a given
> instant I have this:
>
> <pre>
> class Team < ActiveRecord::Base
>   has_many :transfers
>   has_many :players, :through => :transfers,
>     :include => [{ :transfers => :from_end_of_gameweek }, {
:transfers
> => :until_end_of_gameweek }],
>     :conditions => [''gameweeks.start_time <= ? AND
(gameweeks.end_time
> IS NULL OR ? < gameweeks.end_time)'', Time.now.utc,
Time.now.utc]
> end
> </pre>
>
> Thus I can say team.transfers to get a list of all Transfers the Team
> has made, or team.players to get the current squad.
>
> It works, but that has_many is a pretty unpleasant bit of code and the
> SQL doesn''t look the most efficient. The question is, can this be
made
> more elegant and maybe efficient?
>
Are you sure it works ? the gameweeks table is joined twice, so the
second time it will be aliased, ie you''re looking at both the start
and the endtime of the from_end_of_gameweek. on top of that while the
date range condition may work in development (where code is reloaded
on every request) it won''t work in production, the value of Time.now
used will always be what it was when you last restarted you rails app.

A small change you could make is replace the include with :joins =>
{:transfers => [:from_end_of_gameweek, :until_end_of_gameweek]}
This barely changes the sql generated but it means that rails won''t go
off and insantiate objects for the transfers/gameweeks.

You might be able to at least make some slightly more readable code by
having a active_transfers association (those transfers which are
currently in effect) and then having players through that
association.

It doesn''t look to me like the query could be a lot simpler than it is
(as usual having the right indexes can make a world of difference)
Perhaps you could side step it by having appropriate flags on the
transfers table and a cron job that updated those flags at the end of
every gameweek ?

Fred> Thanks.--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

Alan

2009-Feb-07 16:30 UTC

head link

Re: Improving this messy has_many :through with :include

On Feb 7, 1:08 pm, Frederick Cheung
<frederick.che...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:> On Feb 6, 9:53 pm, Alan
<alan.lar...-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>
>
> > I have a question about cleaning up a rather complex has_many.
I''ll
> > describe the context in case it helps, but perhaps you can skip
> > straight to the code below.
>
> > * I am designing a fantasy football type system.
> > * I am modeling a Team which consists of many Players.
> > * I am connecting Teams and Players via a Transfer join model.
> > * A Transfer also has two GameWeeks (a GameWeek encapsulates the
> > start_time and end_time of a period of play).
> > * Transfer.from_end_of_gameweek indicates the GameWeek from which the
> > Player becomes part of the Team.
> > * Transfer.until_end_of_gameweek indicates the GameWeek from which a
> > Player ceases to be part of the Team.
>
> > So Transfers is essentially acting as a warehouse, storing history
> > about all player transfers that have taken place (in the future I
> > might make a dedicated warehouse but not yet).
>
> > Now, to determine which Players are playing for a Team at a given
> > instant I have this:
>
> > <pre>
> > class Team < ActiveRecord::Base
> >   has_many :transfers
> >   has_many :players, :through => :transfers,
> >     :include => [{ :transfers => :from_end_of_gameweek }, {
:transfers
> > => :until_end_of_gameweek }],
> >     :conditions => [''gameweeks.start_time <= ? AND
(gameweeks.end_time
> > IS NULL OR ? < gameweeks.end_time)'', Time.now.utc,
Time.now.utc]
> > end
> > </pre>
>
> > Thus I can say team.transfers to get a list of all Transfers the Team
> > has made, or team.players to get the current squad.
>
> > It works, but that has_many is a pretty unpleasant bit of code and the
> > SQL doesn''t look the most efficient. The question is, can
this be made
> > more elegant and maybe efficient?
>
> Are you sure it works ? the gameweeks table is joined twice, so the
> second time it will be aliased, ie you''re looking at both the
start
> and the endtime of the from_end_of_gameweek. on top of that while the
> date range condition may work in development (where code is reloaded
> on every request) it won''t work in production, the value of
Time.now
> used will always be what it was when you last restarted you rails app.
>
> A small change you could make is replace the include with :joins =>
> {:transfers => [:from_end_of_gameweek, :until_end_of_gameweek]}
> This barely changes the sql generated but it means that rails
won''t go
> off and insantiate objects for the transfers/gameweeks.
>
> You might be able to at least make some slightly more readable code by
> having a active_transfers association (those transfers which are
> currently in effect) and then having players through that
> association.
>
> It doesn''t look to me like the query could be a lot simpler than
it is
> (as usual having the right indexes can make a world of difference)
> Perhaps you could side step it by having appropriate flags on the
> transfers table and a cron job that updated those flags at the end of
> every gameweek ?
>
> Fred
>
> > Thanks.
That Time.now was a bit of a noob mistake, and yes well spotted, some
of the conditions were not using the correct alias. Thanks.

Have reworked and it appears okay now but I am still unhappy because
of hardcoding table aliases. It also peforms an unncessary join.

class Team < ActiveRecord::Base
  has_many :transfers
  has_many :players, :through => :transfers

  def players_during_gameweek(gameweek = GameWeek.current)
    players.find(:all, :include => [{ :transfers =>
[:from_end_of_gameweek, :until_end_of_gameweek] }],
      :conditions => [''gameweeks.end_time <= ? AND
(until_end_of_gameweeks_transfers.end_time IS NULL OR ? <
until_end_of_gameweeks_transfers.end_time)'', gameweek.start_time,
gameweek.start_time])
  end
end
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Talk" group.
To post to this group, send email to
rubyonrails-talk-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
To unsubscribe from this group, send email to
rubyonrails-talk+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-talk?hl=en
-~----------~----~----~----~------~----~------~--~---

Rails - Feb 2009 - Improving this messy has_many :through with :include

Improving this messy has_many :through with :include

Improving this messy has_many :through with :include

Re: Improving this messy has_many :through with :include

Re: Improving this messy has_many :through with :include