Skip to content

Commit

Permalink
Refactor fetch_attribute
Browse files Browse the repository at this point in the history
Similar to the work done in #38636, instead of using case statements we
can make these classes respond to `fetch_attribute`.

New classes can implement `fetch_attribute` instead of adding to the
case statement, it's more object oriented, and nicer looking.

Co-authored-by: Aaron Patterson <aaron.patterson@gmail.com>
  • Loading branch information
eileencodes and tenderlove committed Mar 13, 2020
1 parent 0c715e5 commit ce7bbc3
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 26 deletions.
12 changes: 2 additions & 10 deletions activerecord/lib/arel.rb
Expand Up @@ -47,16 +47,8 @@ def self.arel_node?(value) # :nodoc:
end

def self.fetch_attribute(value, &block) # :nodoc:
case value
when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality,
Arel::Nodes::NotEqual, Arel::Nodes::LessThan, Arel::Nodes::LessThanOrEqual,
Arel::Nodes::GreaterThan, Arel::Nodes::GreaterThanOrEqual
attribute_value = value.detect_attribute
yield attribute_value if attribute_value
when Arel::Nodes::Or
fetch_attribute(value.left, &block) && fetch_attribute(value.right, &block)
when Arel::Nodes::Grouping
fetch_attribute(value.expr, &block)
unless String === value
value.fetch_attribute(&block)
end
end

Expand Down
40 changes: 24 additions & 16 deletions activerecord/lib/arel/nodes/binary.rb
Expand Up @@ -11,14 +11,6 @@ def initialize(left, right)
@right = right
end

def detect_attribute
if self.left.is_a?(Arel::Attributes::Attribute)
self.left
elsif self.right.is_a?(Arel::Attributes::Attribute)
self.right
end
end

def initialize_copy(other)
super
@left = @left.clone if @left
Expand All @@ -37,18 +29,34 @@ def eql?(other)
alias :== :eql?
end

module FetchAttribute
def fetch_attribute
if left.is_a?(Arel::Attributes::Attribute)
yield left
elsif right.is_a?(Arel::Attributes::Attribute)
yield right
end
end
end

class Between < Binary; include FetchAttribute; end
class NotIn < Binary; include FetchAttribute; end
class GreaterThan < Binary; include FetchAttribute; end
class GreaterThanOrEqual < Binary; include FetchAttribute; end
class NotEqual < Binary; include FetchAttribute; end
class LessThan < Binary; include FetchAttribute; end
class LessThanOrEqual < Binary; include FetchAttribute; end

class Or < Binary
def fetch_attribute(&block)
left.fetch_attribute(&block) && right.fetch_attribute(&block)
end
end

%w{
As
Assignment
Between
GreaterThan
GreaterThanOrEqual
Join
LessThan
LessThanOrEqual
NotEqual
NotIn
Or
Union
UnionAll
Intersect
Expand Down
8 changes: 8 additions & 0 deletions activerecord/lib/arel/nodes/equality.rb
Expand Up @@ -10,6 +10,14 @@ def operator; :== end
def invert
Arel::Nodes::NotEqual.new(left, right)
end

def fetch_attribute
if left.is_a?(Arel::Attributes::Attribute)
yield left
elsif right.is_a?(Arel::Attributes::Attribute)
yield right
end
end
end

class IsDistinctFrom < Equality
Expand Down
3 changes: 3 additions & 0 deletions activerecord/lib/arel/nodes/grouping.rb
Expand Up @@ -3,6 +3,9 @@
module Arel # :nodoc: all
module Nodes
class Grouping < Unary
def fetch_attribute(&block)
expr.fetch_attribute(&block)
end
end
end
end
3 changes: 3 additions & 0 deletions activerecord/lib/arel/nodes/node.rb
Expand Up @@ -41,6 +41,9 @@ def to_sql(engine = Table.engine)
collector = engine.connection.visitor.accept self, collector
collector.value
end

def fetch_attribute
end
end
end
end
3 changes: 3 additions & 0 deletions activerecord/lib/arel/nodes/sql_literal.rb
Expand Up @@ -11,6 +11,9 @@ class SqlLiteral < String
def encode_with(coder)
coder.scalar = self.to_s
end

def fetch_attribute
end
end
end
end

0 comments on commit ce7bbc3

Please sign in to comment.