Ok, although not entirely new to RoR, myself and a colleague have been
“discussing” some best practices for find.
The scenario:
- table of users
- table of companies
- a user can belong to multiple companies
- end result is that the list will be used for a select list.
So, if I want to get a list of companies that a particular user has
been with in the last X time frame, which model should have the
specialized find (because we may / may not have to do a “GROUP BY”
as well).
Should it be in the users model, because it’s all the companies would
have the user in common?
Or:
Should it be in the companies model, because it will be a collection
of companies?
Thanks in adavance
Woops … I should’ve also mentioned that the example I gave is quite
a high level rendition.
The reason I’m not using HABTM or another associate is because there
are quite a few other tables between users & companies and some extra
conditions.
Originally we had it all setup through associations but it was just
just too dang slow so doing SQL by hand to significantly speed things
up - the main reason for this is because of the a GROUP BY, ORDER BY
combo we got going on.
But the same principle question applies: which model should be
returning the result?
Based on what you’ve shared, I’d put the finder on Company, since you
want a
set of companies.
@Brent: If the join was that straight forward then there wouldn’t be
such an issue, but as I mentioned in my immediately follow up to the
original post, the join is actually quite complex … the optimized
SQL is as such: (as displayed with checks in the function)
query = "SELECT c.id as company_id, c.name as company_name
FROM entries as e
INNER JOIN project_tasks AS pt ON pt.id =
e.project_task_id
INNER JOIN projects as p ON p.id = pt.project_id
INNER JOIN jobs as j on j.id = p.job_id
INNER JOIN companies as c ON c.id = j.company_id
WHERE e.user_id = “+self.id.to_s+” "
query += " AND entries.start_timestamp >= "+start_timestamp.to_s
+" " if( !start_timestamp.nil? )
query += " AND entries.end_timestamp <= “+end_timestamp.to_s+”
" if( !end_timestamp.nil? )
query += " AND c.active = "+is_active.to_s+"
GROUP BY c.id ORDER BY max( e.start_timestamp ) DESC"
Obviously this more complex than what I originally started polling
about, but this gives insight into the end desired result.
So the poll question remains: should this be in the User model (since
it’s a user’s list of Companies), or be in the Company model since
that’s what it’s actually returning.
On Jan 25, 10:40 am, Brent M. [email protected]
KumaZatheef wrote:
So the poll question remains: should this be in the User model (since
it’s a user’s list of Companies), or be in the Company model since
that’s what it’s actually returning.
In this case, I think it would belong in both places. I would make a
pair of methods, that work together. One in User that asks Company for
a list for itself, and then the method in Company that takes in the
User’s id and returns the list.
You wouldn’t want to put the method just in Users, because it’s stepping
on Company’s toes by pulling data from Company’s table. On the other
hand, you still want to be able to call user.companies.
class Company < ActiveRecord::Base
def self.find_for_user(id)
# excecute the correct SQL, wrapping the results up the ActiveRecord
way
end
end
class User < ActiveRecord::Base
def companies
Company.find_for_user(self.id)
end
end
Hmmm … that’s a good way to cover possible scenarios.
So the “real” call would be in Company, but Users would access it …
is that ok (ex: “standard”) to have one Model accessing another’s
functions … for some reason I figured that’d be crossing into each
other’s territory …
On Jan 25, 2:13 pm, Brent M. [email protected]
KumaZatheef wrote:
is that ok (ex: “standard”) to have one Model accessing another’s
functions … for some reason I figured that’d be crossing into each
other’s territory …
It is standard to have the models talk to each other. That’s what
“has_and_belongs_to_many” and those sorts of relationship declarations
do; they’re just meta-programming tools to create methods like the ones
I’m suggesting.
If you’ve got:
class User < ActiveRecord::Base
has_and_belongs_to_many :companies
end
class Company < ActiveRecord::Base
has_and_belongs_to_many :users
end
All you need to do is:
user = User.find(params[:id])
user.companies
You don’t need to write a special finder: it’s built for you when you
declare the model relationships.
KumaZatheef wrote:
Ok, although not entirely new to RoR, myself and a colleague have been
“discussing” some best practices for find.
The scenario:
- table of users
- table of companies
- a user can belong to multiple companies
- end result is that the list will be used for a select list.
So, if I want to get a list of companies that a particular user has
been with in the last X time frame, which model should have the
specialized find (because we may / may not have to do a “GROUP BY”
as well).
Should it be in the users model, because it’s all the companies would
have the user in common?
Or:
Should it be in the companies model, because it will be a collection
of companies?
Thanks in adavance
oh yeah, right … duh.
That makes sense, thanks a bunch for your feedback, much appreciated!!
I’m curious what others have to say as well … agree / disagree?
On Jan 25, 4:30 pm, Brent M. [email protected]
KumaZatheef wrote:
oh yeah, right … duh.
That makes sense, thanks a bunch for your feedback, much appreciated!!
I’m curious what others have to say as well … agree / disagree?
On Jan 25, 4:30 pm, Brent M. [email protected]
Disagree:
I would put that nasty looking query in the association itself. Sence
that is something that is probably going to be a maintenance issue all
by itsself, Id make it a module – esp. because I bet you have more then
one of these functions:
#UserCompanyModule.rb
Module UserCompanyAssociationMethods
def been_at_sense(time) #or whatever
#code here to get the right companies
end
#more methods here
end
class User
has_and_belongs_to_many :companies,
:extend=>UserCompanyAssociationMethods
end
#somewhere in your code
user = User.find(…)
companies = user.companies.been_at_sense(Time.now-2.years)
As an aside, I think that habtm relationships are only useful in very
simple circumstances. If the relationship is that complex you should
use has_many and put some of the logic in joining tables eg:
User: has_many :employments
Company: has_many :employments
Employment: belongs_to :user; belongs_to :company
In any case, put these functions into models and use them to extend the
associations themselves. Easier to test, Easier to maintain.
Hmmm … interesting idea of putting it in a Module, never thought of
that.
I’ll take a look at the feasibility of doing it that way, within the
context of our existing code.
You’re right, we do have more than one of these functions; however,
the others apply to associations between User and other Models (ex:
jobs of a given company).
Any other thoughts / ideas out there?
I’m liking all of these suggestions so far … thank you so much to
those that have contributed, very helpful, much appreciated!
On Jan 25, 6:54 pm, Starr T. [email protected]