bmuller/twistar

about delete() and deleteAll()

Closed this issue · 4 comments

Hi,
I'm trying to add a beforeDelete method to the delete chain. In my branch you can see what I've done.
The beforeDelete is fully usable on delete() but I'd like know what you think about adding it on deleteAll(). If I modify deleteAll I need to istantiate all the DBObjects before deletion and call on everyone the delete(). This will degrade performances... is it ok for you?

During my experiments I noticed that HABTM jointable is not cleared when an Object involved in this type of relation is deleted. I have fixed this but the code is mixed with the beforeDelete stuff.

bye

I don't think it's a good idea to instantiate every object and call beforeDelete on each object. Other APIs (see http://api.rubyonrails.org/classes/ActiveRecord/Relation.html#method-i-delete_all for instance) don't seem to do it. I think deleteAll methods are generally just shortcuts to SQL delete statements more than anything else, and I'd hate to do more than generate and execute the corresponding SQL (and hurt performance).

ok! So it's better to add a destroy and destroyall to do what I thought... Is it ok?

Yeah - what you did is fine - I'm going to merge your patches and add a test or two - both for the beforeDelete and the HABTM jointable fix.

Thanks for the contribution!
-B

Merged. Thanks again!