There's got to be a better way!

Hi,

I have an array of contact ids and an array of group ids. I’m trying to
loop
through all the contacts and add them to the group if they aren’t
already in
the group. The relationship is as follows:

class Contact < ActiveRecord::Base
has_many :contact_groups
has_many :groups, :through => :contact_groups
end

class ContactGroup < ActiveRecord::Base
belongs_to :contact
belongs_to :group
end

class Group < ActiveRecord::Base
has_many :contact_groups
has_many :contacts, :through => :contact_groups
end

Here’s what I’m currently doing which works. However, the code is
clunky.
It’s the has_many :through relationship that’s throwing me for a loop…
literally:

for contact in params[:contact]
for group in params[:group]
ContactGroup.find_or_create_by_contact_id_and_group_id(contact,
group)
end
end

Is there a better way?

Thanks!
-Dave

Given your problem statement:

I’m trying to loop through all the contacts and add them to the group if they
aren’t already in the group.

And assuming your models/associations as defined above, and:

@group = Group.find(1)

Then this should loop through all contacts and add to the group any that
aren’t already in the group:

Contact.find(:all).each {|c| @group.contacts << c unless
@group.contacts.include?©}

c.

Hi Cayce,

I tried your suggestion but I keep receiving an error that reads:

undefined method `contacts’ for #Array:0x246be0c

This is how I implemented your suggestion:

@group = Group.find(params[:group])
Contact.find(params[:contact]).each {|c| @group.contacts << c unless @
group.contacts.include?©}

params[:group] and params[:contact] hold an array of IDs that originate
from
a series of check boxes.

I’m pretty sure the relationships are set up right, so I’m kind of lost
as
to why this undefined method error is occurring.

Thanks,
Dave

Looks like Group.find(params[:group]) is returning an Array rather than
a single instance of Group. To use Cayce’s algorithm (nicely done, by
the way), @group will need to represent a single instance of Group

Cheers.

Yep, that’s definitely the problem. Initially, I’d have said try this…

Group.find(params[:group]).each do |g|
Contact.find(:all).each {|c| g.contacts << c unless
g.contacts.include?©}
end

Only - this is going through all contacts in the database and adding
those that don’t exist in the group. I’m gathering from your second post
that what you’re after is to add all contacts in params[:contact] to all
groups in params[:group]. In that case, you just need to change the
Contact.find() parms - this should work for you…

Group.find(params[:group]).each do |g|
Contact.find(params[:contact]).each {|c| g.contacts << c unless
g.contacts.include?©}
end

Enjoy.

c.

Dave H. wrote:

Hi Cayce,

I tried your suggestion but I keep receiving an error that reads:

undefined method `contacts’ for #Array:0x246be0c

This is how I implemented your suggestion:

@group = Group.find(params[:group])
Contact.find(params[:contact]).each {|c| @group.contacts << c unless @
group.contacts.include?©}

params[:group] and params[:contact] hold an array of IDs that originate
from
a series of check boxes.

I’m pretty sure the relationships are set up right, so I’m kind of lost
as
to why this undefined method error is occurring.

Thanks,
Dave

Looks like Group.find(params[:group]) is returning an Array rather than
a single instance of Group. To use Cayce’s algorithm (nicely done, by
the way), @group will need to represent a single instance of Group

Cheers.

Cayce,

That worked like a charm! Learning Rails before really learning Ruby
makes
some things harder than what they really should be.

David,

I think it’s a good idea to eagerly load the contacts like you
suggested.

Thanks to all that helped!
-Dave

Hi –

On Fri, 10 Nov 2006, Cayce B. wrote:

Group.find(params[:group]).each do |g|
Contact.find(params[:contact]).each {|c| g.contacts << c unless
g.contacts.include?(c)}
end

I think you can save a lot of database hits if you load the contacts
eagerly:

Group.find(params[:group], :include => “contacts”)

Otherwise every reference to g.contacts is going to have to go back to
the database.

David


David A. Black | [email protected]
Author of “Ruby for Rails” [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB’s Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] Ruby for Rails | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org

I think you can save a lot of database hits if you load the contacts
eagerly:

Group.find(params[:group], :include => “contacts”)

Otherwise every reference to g.contacts is going to have to go back to
the database.

David


David A. Black | [email protected]
Author of “Ruby for Rails” [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB’s Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] Ruby for Rails | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org

+1 on that.

c.

Hi –

On Fri, 10 Nov 2006, Dave H. wrote:

Cayce,

That worked like a charm! Learning Rails before really learning Ruby makes
some things harder than what they really should be.

That’s why you should pick up “Ruby for Rails” :slight_smile: (No, really! It’s
true! Self-serving, but true :slight_smile:

David


David A. Black | [email protected]
Author of “Ruby for Rails” [1] | Ruby/Rails training & consultancy [3]
DABlog (DAB’s Weblog) [2] | Co-director, Ruby Central, Inc. [4]
[1] Ruby for Rails | [3] http://www.rubypowerandlight.com
[2] http://dablog.rubypal.com | [4] http://www.rubycentral.org

Dave H. wrote:

I actually do own that book (and I thank you for writing it!). There’s
so
much info in there. Takes awhile to get through it :slight_smile:

Well, this is not at all self serving.

Let me just say that being on my second time through David’s book
(amazing the things that you pick up on once you have a bit of
undertanding under your belt.) and having attended a week long bootcamp
that he taught, the book really should be required reading.

I actually do own that book (and I thank you for writing it!). There’s
so
much info in there. Takes awhile to get through it :slight_smile: