Delete_if doesn't work for has_and_belongs_to_many

I’m using Rails-1.1.4 with Ruby-1.8.4

So let’s say I have 2 model classes:

class Item
has_and_belongs_to_many :things
end

class Thing
has_and_belongs_to_many :items
end

i = Item.new
i.things << Thing.new( :value => “Something”).save
i.things << Thing.new( :value => “Something Else” ).save

i.save

Fine, now I have two records in the items_things table. Now if I say

t = Thing.find_by_value ‘Something’
i.things.delete t
i.save

Then I have only one record in the items_things table. Which is correct.
But, if I had said

i.things.delete_if {|x| x.value = ‘Something’}
i.save

then the relevant record is not deleted from the items_things table.

Is this a bug? If so, where’s the best place to report it?

bye
John

Hi –

On Tue, 8 Aug 2006, John A. wrote:

end
i.things.delete t
Is this a bug? If so, where’s the best place to report it?
There’s no delete_if method defined for ActiveRecord collections,
other than the regular Array#delete_if, so it’s not a bug. It could
be a candidate for a patch, I imagine.

David


http://www.rubypowerandlight.com => Ruby/Rails training & consultancy
----> SEE SPECIAL DEAL FOR RUBY/RAILS USERS GROUPS! <-----
http://dablog.rubypal.com => D[avid ]A[. ]B[lack’s][ Web]log
Ruby for Rails => book, Ruby for Rails
http://www.rubycentral.org => Ruby Central, Inc.

On Tue, 2006-08-08 at 08:39 -0400, [email protected] wrote:

There’s no delete_if method defined for ActiveRecord collections,
other than the regular Array#delete_if, so it’s not a bug. It could
be a candidate for a patch, I imagine.

Ah, OK. What was the rationale behind implementing delete but not
delete_if?

bye
John

Hi –

On Tue, 8 Aug 2006, John A. wrote:

There’s no delete_if method defined for ActiveRecord collections,
other than the regular Array#delete_if, so it’s not a bug. It could
be a candidate for a patch, I imagine.

Ah, OK. What was the rationale behind implementing delete but not
delete_if?

delete_all and/or destroy_all, with conditions, should serve pretty
much the same purpose.

David


http://www.rubypowerandlight.com => Ruby/Rails training & consultancy
----> SEE SPECIAL DEAL FOR RUBY/RAILS USERS GROUPS! <-----
http://dablog.rubypal.com => D[avid ]A[. ]B[lack’s][ Web]log
Ruby for Rails => book, Ruby for Rails
http://www.rubycentral.org => Ruby Central, Inc.

That would be association_collection.rb

But I’m not sure you want methods like delete_if and friends to
actually delete the objects from the database. You still need to be
able to manipulate an array of records as just that, an array. There
has to be a balance.

-Jonathan.

On Tue, 2006-08-08 at 09:20 -0400, [email protected] wrote:

Is this a bug? If so, where’s the best place to report it?

There’s no delete_if method defined for ActiveRecord collections,
other than the regular Array#delete_if, so it’s not a bug. It could
be a candidate for a patch, I imagine.

Ah, OK. What was the rationale behind implementing delete but not
delete_if?

delete_all and/or destroy_all, with conditions, should serve pretty
much the same purpose.

It’s the ORM mismatch again - delete_if works with objects whereas
delete_all works with records. Looks like it would be fairly simple to
implement delete_if in terms of delete_all though? Any idea what file I
could start looking at?

bye
John

On Wed, 2006-08-09 at 02:47 +1200, Jonathan V. wrote:

That would be association_collection.rb

But I’m not sure you want methods like delete_if and friends to
actually delete the objects from the database. You still need to be
able to manipulate an array of records as just that, an array. There
has to be a balance.

OK, let me play Devil’s Advocate then. What would the use case be where
one wants to delete an element from the array without deleting the
corresponding record from the db?

bye
John

Hi –

On Tue, 8 Aug 2006, John A. wrote:

corresponding record from the db?
Back at ya’:

Given that there are various operations that can be performed,
including just manipulating the array/collection in memory, why should
AR have to choose one operation from those available, and make as many
constructs as possible perform it? Why does there have to be a
“winner”, when the situation is already such that there’s at least one
way to do all of the things in question?

David


http://www.rubypowerandlight.com => Ruby/Rails training & consultancy
----> SEE SPECIAL DEAL FOR RUBY/RAILS USERS GROUPS! <-----
http://dablog.rubypal.com => D[avid ]A[. ]B[lack’s][ Web]log
Ruby for Rails => book, Ruby for Rails
http://www.rubycentral.org => Ruby Central, Inc.

On Tue, 2006-08-08 at 14:22 -0400, [email protected] wrote:

OK, let me play Devil’s Advocate then. What would the use case be where
one wants to delete an element from the array without deleting the
corresponding record from the db?

Back at ya’:

Given that there are various operations that can be performed,
including just manipulating the array/collection in memory,

Which is what Array is for, right? ActiveRecord exists precisely because
of a need to manipulate objects (well, records ultimately) in db tables.

why should
AR have to choose one operation from those available, and make as many
constructs as possible perform it?

Well, I figured that based on the behaviour with delete, I could safely
assume that delete_if (since it didn’t throw a missing method exception)
would do the same kind of thing. I was surprised when they behaved
differently.

Why does there have to be a “winner”,

I’m not sure what you mean here?

when the situation is already such that there’s at least one
way to do all of the things in question?

From my point of view, another way of seeing it is this: why is there a
method that does something very unexpected (ie not manipulate records in
the db when that’s the express purpose of the collection in question),
when it doesn’t even need to exist since there are other ways to achieve
the same functionality?

And if one was in the mood for making tangential, fine and probably
mostly irrelevant :wink: distinctions, one could say that delete_if allows
one to manipulate the collection from an object point of view, whereas
delete_all allows one to manipulate the collection from an SQL point of
view.

bye
John

Given that there are various operations that can be performed,
including just manipulating the array/collection in memory, why should
AR have to choose one operation from those available, and make as many
constructs as possible perform it? Why does there have to be a
“winner”, when the situation is already such that there’s at least one
way to do all of the things in question?

I think what David is saying (because it’s a bit cryptic) is that you
already have the functionality you need via model#delete(:conditions =>
[]) without having to step on the toes of other, original array methods.

Given this, why bother with forcing ActiveRecord to chew up more methods
than is needed? (My co-worker does this kind of stuff and it irks me to
no end. It’s called #update_attributes you fool! Sorry. :slight_smile:

  • Daniel

On Tue, 2006-08-08 at 21:44 +0200, Daniel W. wrote:

Given that there are various operations that can be performed,
including just manipulating the array/collection in memory, why should
AR have to choose one operation from those available, and make as many
constructs as possible perform it? Why does there have to be a
“winner”, when the situation is already such that there’s at least one
way to do all of the things in question?

I think what David is saying (because it’s a bit cryptic) is that you
already have the functionality you need via model#delete(:conditions =>
[])

Not at all. That was clear to me.

without having to step on the toes of other, original array methods.

That’s exactly what I find puzzling. For me it’s not stepping on the
toes of other methods, it’s making sure that if you’re masquerading as
someone else then you do a proper job.

For Array, delete_if is a more general form of delete - I can implement
the delete functionality in terms of delete_if (Well, except for the
returning of the block if the item isn’t found). But I can’t do that
with an AR collection. That strikes me as very odd.

Given this, why bother with forcing ActiveRecord to chew up more methods
than is needed?

Because if you don’t do a good job masquerading as someone else, someone
will see through your disguise.

In this case AssociationProxy implements method_missing and forwards
delete_if to its underlying Array.

(My co-worker does this kind of stuff and it irks me to
no end. It’s called #update_attributes you fool! Sorry. :slight_smile:

What kind of stuff? And what irks you, exactly?

bye
John

Hi –

On Tue, 8 Aug 2006, John A. wrote:

From my point of view, another way of seeing it is this: why is there a
method that does something very unexpected (ie not manipulate records in
the db when that’s the express purpose of the collection in question),
when it doesn’t even need to exist since there are other ways to achieve
the same functionality?

It’s interesting that push is magic, but pop isn’t :slight_smile: And same with
delete and delete_if. Also, while you can’t do:

ar_collection << 100

you can do:

ar_collection[1] = 100

I guess it’s a tradeoff between the vanilla array behavior and the
special AR behavior – though in some of these cases a case could
certainly be made for some kind of unification.

David


http://www.rubypowerandlight.com => Ruby/Rails training & consultancy
----> SEE SPECIAL DEAL FOR RUBY/RAILS USERS GROUPS! <-----
http://dablog.rubypal.com => D[avid ]A[. ]B[lack’s][ Web]log
Ruby for Rails => book, Ruby for Rails
http://www.rubycentral.org => Ruby Central, Inc.