Search code examples
rubymetricsstatic-analysisrubocop

Is there a better way to assign a Ruby hash while avoiding RuboCop's ABC Size warnings?


I have a method that builds a laptop's attributes, but only if the attributes are present within a row that is given to the method:

def build_laptop_attributes desk_id, row, laptop
  attributes = {}
  attributes[:desk_number] = room_id if laptop && desk_id
  attributes[:status] = row[:state].downcase if row[:state]
  attributes[:ip_address] = row[:ip_address] if row[:ip_address]
  attributes[:model] = row[:model] if row[:model]
  attributes
end

Currently, RuboCop is saying that the Metric/AbcSize is too high, and I was wondering if there is an obvious and clean way to assign these attributes?


Solution

  • Style Guides Provide "Best Practices"; Evaluate and Tune When Needed

    First of all, RuboCop is advisory. Just because RuboCop complains about something doesn't mean it's wrong in some absolute sense; it just means you ought to expend a little more skull sweat (as you're doing) to see if what you're doing makes sense.

    Secondly, you haven't provided a self-contained, executable example. That makes it impossible for SO readers to reliably refactor it, since it can't currently be tested without sample inputs and expected outputs not provided in your original post. You'll need those things yourself to evaluate and refactor your own code, too.

    Finally, the ABC Metric looks at assignments, branches, and conditionals. You have five assignments, four conditionals, and what looks liks a method call. Is that a lot? If you haven't tuned Rubocop, the answer is "RuboCop thinks so." Whether or not you agree is up to you and your team.

    If you want to try feeding Rubocop, you can do a couple of things that might help reduce the metric:

    1. Refactor the volume and complexity of your assignments. Some possible examples include:
      • Replace your postfix if-statements with safe navigators (&.) to guard against calling methods on nil.

      • Extract some of your branching logic and conditionals to methods that "do the right thing", potentially reducing your current method to a single assignment with four method calls. For example:

        attributes = { desk_number: location, status: laptop_status, ... }
        
      • Replace all your multiple assignments with a deconstructing assignment (although Rubocop often complains about those, too).

    2. Revisit whether you have the right data structure in the first place. Maybe you really just want an OpenStruct, or some other data object.

    Your current code seems readable, so is the juice really worth the squeeze? If you decide that RuboCop is misguided in this particular case, and your code works and passes muster in your internal code reviews, then you can tune the metric's sensitivity in your project's .rubocop.yml or disable that particular metric for just that section of your source code.