Created
December 6, 2018 17:16
-
-
Save Supernats/d3226162342ba7d484580944b565ab54 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
require "ostruct" | |
module RubocopHashLiteral | |
class Literal | |
def initialize(data) | |
@data = data | |
end | |
def to_h | |
{ | |
address_street1: data.address.street1, | |
address_street2: data.address.street2, | |
address_city: data.address.city, | |
address_state: data.address.state, | |
address_zip: data.address.zip, | |
best_friend_address_street1: data.best_friend.address.street1, | |
best_friend_address_street2: data.best_friend.address.street2, | |
best_friend_address_city: data.best_friend.address.city, | |
best_friend_address_state: data.best_friend.address.state, | |
best_friend_address_zip: data.best_friend.address.zip, | |
} | |
end | |
private | |
attr_reader :data | |
end | |
class Iterative | |
KEYS = %i[ | |
address_street1 | |
address_street2 | |
address_city | |
address_state | |
address_zip | |
best_friend_address_street1 | |
best_friend_address_street2 | |
best_friend_address_city | |
best_friend_address_state | |
best_friend_address_zip | |
].freeze | |
private_constant :KEYS | |
def initialize(data) | |
@data = data | |
end | |
def to_h | |
Hash[ | |
KEYS.zip( | |
KEYS.map do |message| | |
decorator.public_send(message) | |
end, | |
) | |
] | |
end | |
private | |
attr_reader :data | |
def decorator | |
@decorator ||= Decorator.new(data) | |
end | |
class Decorator | |
def initialize(data) | |
@data = data | |
end | |
def address_street1 | |
data.address.street1 | |
end | |
def address_street2 | |
data.address.street2 | |
end | |
def address_city | |
data.address.city | |
end | |
def address_state | |
data.address.state | |
end | |
def address_zip | |
data.address.zip | |
end | |
def best_friend_address_street1 | |
data.best_friend.address.street1 | |
end | |
def best_friend_address_street2 | |
data.best_friend.address.street2 | |
end | |
def best_friend_address_city | |
data.best_friend.address.city | |
end | |
def best_friend_address_state | |
data.best_friend.address.state | |
end | |
def best_friend_address_zip | |
data.best_friend.address.zip | |
end | |
private | |
attr_reader :data | |
end | |
end | |
class DataHashTest | |
class << self | |
delegate :call, to: :new | |
end | |
def call | |
iterative_hash == literal_hash | |
end | |
private | |
def data | |
@data ||= Provider.new | |
end | |
def iterative_hash | |
@iterative_hash ||= Iterative.new(data).to_h | |
end | |
def literal_hash | |
@literal_hash ||= Literal.new(data).to_h | |
end | |
end | |
class Provider | |
class << self | |
def new | |
OpenStruct.new( | |
address: OpenStruct.new( | |
street1: :street1, | |
street2: :street2, | |
city: :city, | |
state: :state, | |
zip: :zip, | |
), | |
best_friend: OpenStruct.new( | |
address: OpenStruct.new( | |
street1: :street1, | |
street2: :street2, | |
city: :city, | |
state: :state, | |
zip: :zip, | |
), | |
), | |
) | |
end | |
end | |
end | |
end |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Avoid high complexity on large hash literals
tl;dr
RubocopHashLiteral::Literal#to_h
is the way we've often done thingsRubocopHashLiteral::Iterative#to_h
is a new way we could do thingsRubocopHashLiteral::DataHashTest::call
will let you prove they'retruly equivalent
This change is put up as a sketch of what we can do when we hit the
complexity cop on large hash literals, which I have seen a fair number
of times.
RuboCop does not like high ABC complexity. Law of Demeter violations are
a very good way to drive that complexity up. In this file,
Literal#to_h
has an ABC complexity of 35, which is 20 over the 15 ourRuboCop configuration lists as the max.
The issue is that, while it just seems like we're making a dumb
configuration literal, we've actually encoded quite a few dependencies
into each line, but since it's just setting a key-value pair we maybe
don't think of it that way. The Law of Demeter is also known as the
Principle of Least Knowledge, which I feel makes much more clear the
solution to take. Swap out that knowledge of the data structure with an
object that lets us send messages about exactly what we want, with no
knowledge of how it's furnished to us.
While this definitely increases the size of the deployed code, I would
say that size is now a much more fair representation of the complexity
inherent in what we're doing.