Permalink
Browse files

Replace inline lambdas with named methods

  • Loading branch information...
1 parent bdd549a commit df63c99f0602b1854870e6176a144358021b5014 @jonleighton jonleighton committed Jun 12, 2011
Showing with 42 additions and 29 deletions.
  1. +42 −29 activerecord/lib/active_record/associations/collection_association.rb
@@ -114,21 +114,13 @@ def create!(attributes = {}, options = {}, &block)
# Add +records+ to this association. Returns +self+ so method calls may be chained.
# Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically.
def concat(*records)
- result = true
load_target if owner.new_record?
- block = lambda do
- records.flatten.each do |record|
- raise_on_type_mismatch(record)
- add_to_target(record) do |r|
- result &&= insert_record(record) unless owner.new_record?
- end
- end
+ if owner.new_record?
+ concat_records(records)
+ else
+ transaction { concat_records(records) }
end
-
- owner.new_record? ? block.call : transaction(&block)
-
- result && records
end
# Starts a transaction in the association class's database connection.
@@ -297,17 +289,11 @@ def replace(other_array)
other_array.each { |val| raise_on_type_mismatch(val) }
original_target = load_target.dup
- block = lambda do
- delete(target - other_array)
-
- unless concat(other_array - target)
- @target = original_target
- raise RecordNotSaved, "Failed to replace #{reflection.name} because one or more of the " \
- "new records could not be saved."
- end
+ if owner.new_record?
+ replace_records(other_array, original_target)
+ else
+ transaction { replace_records(other_array, original_target) }
end
-
- owner.new_record? ? block.call : transaction(&block)
end
def include?(record)
@@ -448,16 +434,20 @@ def delete_or_destroy(records, method)
records.each { |record| raise_on_type_mismatch(record) }
existing_records = records.reject { |r| r.new_record? }
- block = lambda do
- records.each { |record| callback(:before_remove, record) }
+ if existing_records.empty?
+ remove_records(existing_records, records, method)
+ else
+ transaction { remove_records(existing_records, records, method) }
+ end
+ end
- delete_records(existing_records, method) if existing_records.any?
- records.each { |record| target.delete(record) }
+ def remove_records(existing_records, records, method)
+ records.each { |record| callback(:before_remove, record) }
- records.each { |record| callback(:after_remove, record) }
- end
+ delete_records(existing_records, method) if existing_records.any?
+ records.each { |record| target.delete(record) }
- existing_records.any? ? transaction(&block) : block.call
+ records.each { |record| callback(:after_remove, record) }
end
# Delete the given records from the association, using one of the methods :destroy,
@@ -466,6 +456,29 @@ def delete_records(records, method)
raise NotImplementedError
end
+ def replace_records(new_target, original_target)
+ delete(target - new_target)
+
+ unless concat(new_target - target)
+ @target = original_target
+ raise RecordNotSaved, "Failed to replace #{reflection.name} because one or more of the " \
+ "new records could not be saved."
+ end
+ end
+
+ def concat_records(records)
+ result = true
+
+ records.flatten.each do |record|
+ raise_on_type_mismatch(record)
+ add_to_target(record) do |r|
+ result &&= insert_record(record) unless owner.new_record?
+ end
+ end
+
+ result && records
+ end
+
def callback(method, record)
callbacks_for(method).each do |callback|
case callback

0 comments on commit df63c99

Please sign in to comment.