# Improving this messy has\_many :through with :include

**URL:** <https://www.ruby-forum.com/t/improving-this-messy-has-many-through-with-include/196893>\
**Category:** Rails\
**Created:** [February 7, 2009, 1:45pm UTC](https://www.ruby-forum.com/t/improving-this-messy-has-many-through-with-include/196893 "2009-02-07T13:45:39Z")\
**Posts on this page:** 3\
**Page:** 1

<div class="post-metadata">

**Author:** ![alan](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/a/a87d85/32.png) [@alan](https://www.ruby-forum.com/u/alan)\
**Post date:** [February 7, 2009, 1:45pm UTC](https://www.ruby-forum.com/t/improving-this-messy-has-many-through-with-include/196893/1 "2009-02-07T13:45:39Z")

</div>

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:

```
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
```

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.

---

<div class="post-metadata">

**Author:** ![alan](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/a/a87d85/32.png) [@alan](https://www.ruby-forum.com/u/alan)\
**Post date:** [February 7, 2009, 2:09pm UTC](https://www.ruby-forum.com/t/improving-this-messy-has-many-through-with-include/196893/2 "2009-02-07T14:09:41Z")

</div>

On Feb 6, 9:53 pm, Alan [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid) wrote:

> Player becomes part of the Team.
> 
> ```
> 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

---

<div class="post-metadata">

**Author:** ![alan](https://www.ruby-forum.com/letter_avatar_proxy/v4/letter/a/a87d85/32.png) [@alan](https://www.ruby-forum.com/u/alan)\
**Post date:** [February 7, 2009, 5:30pm UTC](https://www.ruby-forum.com/t/improving-this-messy-has-many-through-with-include/196893/3 "2009-02-07T17:30:52Z")

</div>

On Feb 7, 1:08 pm, Frederick C. [removed\_email\_address@domain.invalid](mailto:removed_email_address@domain.invalid)  
wrote:

> > - I am connecting Teams and Players via a Transfer join model.
> 
> > IS NULL OR ? \< gameweeks.end\_time)', Time.now.utc, Time.now.utc]  
> > Are you sure it works ? the gameweeks table is joined twice, so the
> 
> 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