Browse files

These extra array operations appear to be unnecessary. Reasoning:

 * It is not necessary to subtract 'id' from the list of copied attributes because record and mem_record are equal, so therefore their id attributes are also equal (so there is no harm in copying it, and this reduces the complexity of the code)
 * It is not necessary to intersect the attribute names, since record and mem_record are equal, so they have the same id and class, so they have the same columns in the database. If record has non-column attributes then it seems reasonable to also copy them onto mem_record (though I am not sure what situation this would ever happen in)
  • Loading branch information...
1 parent 60b23ea commit 7e6145b4a2108a4ad94af02c3070789c03ccbd00 @jonleighton jonleighton committed May 14, 2011
Showing with 2 additions and 8 deletions.
  1. +2 −8 activerecord/lib/active_record/associations/collection_association.rb
@@ -405,14 +405,8 @@ def merge_target_lists(persisted, memory)
mem_record = memory.delete(record)
if mem_record
- # Only try to assign attributes which exist on mem_record
- shared = mem_record.attribute_names & record.attribute_names
- # Don't try to assign the primary key, or attributes which have changed on mem_record
- excluded = ["id"] + mem_record.changes.keys
- (shared - excluded).each do |key|
- mem_record[key] = record[key]
+ (record.attribute_names - mem_record.changes.keys).each do |name|
+ mem_record[name] = record[name]

0 comments on commit 7e6145b

Please sign in to comment.