Permalink
Browse files

Extract conditional to a method to avoid duplication

Also use if/else block to not use short circuit return
  • Loading branch information...
rafaelfranca committed Jun 19, 2012
1 parent 1f98493 commit 60ffe41980f3989637bdf537341288288aa9cc39
@@ -108,7 +108,7 @@ def calculate(operation, column_name, options = {})
if relation.equal?(self)
- if eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?))
+ if has_include?(column_name)
construct_relation_for_association_calculations.calculate(operation, column_name, options)
else
perform_calculation(operation, column_name, options)
@@ -156,25 +156,25 @@ def pluck(column_name)
column_name = "#{table_name}.#{column_name}"
end
- if eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?))
- return construct_relation_for_association_calculations.pluck(column_name)
- end
-
- result = klass.connection.select_all(select(column_name).arel, nil, bind_values)
+ if has_include?(column_name)
+ construct_relation_for_association_calculations.pluck(column_name)
+ else
+ result = klass.connection.select_all(select(column_name).arel, nil, bind_values)
- key = result.columns.first
- column = klass.column_types.fetch(key) {
- result.column_types.fetch(key) {
- Class.new { def type_cast(v); v; end }.new
+ key = result.columns.first
+ column = klass.column_types.fetch(key) {
+ result.column_types.fetch(key) {
+ Class.new { def type_cast(v); v; end }.new
+ }
}
- }
- result.map do |attributes|
- raise ArgumentError, "Pluck expects to select just one attribute: #{attributes.inspect}" unless attributes.one?
+ result.map do |attributes|
+ raise ArgumentError, "Pluck expects to select just one attribute: #{attributes.inspect}" unless attributes.one?
- value = klass.initialize_attributes(attributes).values.first
+ value = klass.initialize_attributes(attributes).values.first
- column.type_cast(value)
+ column.type_cast(value)
+ end
end
end
@@ -190,6 +190,10 @@ def ids
private
+ def has_include?(column_name)
+ eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?))
+ end
+
def perform_calculation(operation, column_name, options = {})
operation = operation.to_s.downcase
@@ -423,6 +423,7 @@ def test_maximum_with_from_option
def test_maximum_with_not_auto_table_name_prefix_if_column_included
Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)])
+ # TODO: Investigate why PG isn't being typecast
if current_adapter?(:PostgreSQLAdapter)
assert_equal "7", Company.includes(:contracts).maximum(:developer_id)
else
@@ -433,6 +434,7 @@ def test_maximum_with_not_auto_table_name_prefix_if_column_included
def test_minimum_with_not_auto_table_name_prefix_if_column_included
Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)])
+ # TODO: Investigate why PG isn't being typecast
if current_adapter?(:PostgreSQLAdapter)
assert_equal "7", Company.includes(:contracts).minimum(:developer_id)
else
@@ -443,6 +445,7 @@ def test_minimum_with_not_auto_table_name_prefix_if_column_included
def test_sum_with_not_auto_table_name_prefix_if_column_included
Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)])
+ # TODO: Investigate why PG isn't being typecast
if current_adapter?(:MysqlAdapter) || current_adapter?(:PostgreSQLAdapter)
assert_equal "7", Company.includes(:contracts).sum(:developer_id)
else

0 comments on commit 60ffe41

Please sign in to comment.