Ruby method refactor multi line assignment to pass RuboCop

Question

I have been trying to tune this method that sets up complex assignment and I am looking for other options to make this function pass the cops.

Would anyone have thoughts to point me in the right direction?

Right now, I am tinkering with breaking out the two inner .map calls.

Failing Cops

Assignment Branch Condition size for parse_items is too high. [24.08/15]
  def parse_items
Avoid multi-line chains of blocks.
    end.compact.map do |opt|

The problem code

  def parse_items
    options = parse_relationships
    options = options.select { |opt| opt['type'] == 'product_options' }
    options.map do |opt|
      parse_included.detect { |x| x['id'] == opt['id'] }
    end.compact.map do |opt|
      {
        group_id: @payload['id'],
        originator_id: opt['id'],
        price: opt['attributes']['price'],
        description: opt['attributes']['name'],
        exp_quantity: opt['attributes']['quantity'].to_i,
        title: parse_attributes['name'],
        image_originator_url: 'image_for_product',
        updated_at: timestamp
      }
    end
  end

Helper Methods

  private

  def parse_data
    @payload['data']
  rescue
    []
  end

  def parse_included
    @payload['included']
  rescue
    []
  end

  def parse_attributes
    @payload['data']['attributes']
  rescue
    []
  end

  def parse_relationships
    @payload['data']['relationships']['options']['data']
  rescue
    []
  end

  def timestamp
    Time.parse(parse_attributes['updated_at'])
  end

Updated Errors

In the spec: wrong number of arguments (given 2, expected 1) for Failure/Error: SELECT = ->(opt) { opt['type'] == 'product_options' }

Assignment Branch Condition size for parse_items is too high. [17/15]

Updated Code

  SELECT = ->(opt) { opt['type'] == 'product_options' }
  MAP = ->(opt) { parse_included.detect { |x| x['id'] == opt['id'] } }
  def parse_items
    parse_relationships.select(&SELECT).map(&MAP).compact.map do |opt|
      {
        group_id: @payload['id'],
        originator_id: opt['id'],
        price: opt['attributes']['price'],
        description: opt['attributes']['name'],
        exp_quantity: opt['attributes']['quantity'].to_i,
        title: parse_attributes['name'],
        image_originator_url: 'image_for_product',
        updated_at: timestamp
      }
    end
  end

Put `# rubocop:disable ...` / `# rubocop:enable ...` around this method. Rubocop is good to suggest, but her recommendations are not graved in stone. — Aleksei Matiushkin, Aug 10 '16 at 17:02
I get that, and it is kinda the easy way out. I would rather see if there are different ways to accomplish this. — Chris Hough, Aug 10 '16 at 17:03

score 0 · Accepted Answer · answered Aug 10 '16 at 20:44

I was able to refactor this making it far cleaner and pass all the cops! Hooray!

  def parse_items
    assign_item_attributes(select_included_option(select_item_options(parse_relationships['options']['data'])))
  end

  def select_included_option(options)
    options.map do |opt|
      parse_included.detect { |x| x['id'] == opt['id'] }
    end
  end

  def assign_item_attributes(options)
    options.compact.map do |opt|
      {
        group_id: @payload['id'],
        originator_id: opt['id'],
        price: opt['attributes']['price'],
        description: opt['attributes']['name'],
        exp_quantity: opt['attributes']['quantity'].to_i,
        title: parse_attributes['name'],
        image_originator_url: parse_image,
        updated_at: parse_timestamp
      }
    end
  end

  def select_item_options(options)
    options.select { |opt| opt['type'] == 'product_options' }
  end

This code now looks like FORTRAN written with Ruby syntax. You just flew back 40 years using Rubocop Satistaction Time Machine. We (hopefully) ran out of an era of procedural programming and this code is one of worst examples of it (`assign_item_attributes(select_included_option(select_item_options(parse_relationships['options']['data'])))`.) I downvoted because it is enormously huge step back comparing to what you’ve had before. — Aleksei Matiushkin, Aug 11 '16 at 08:48
I collaborated with another senior on this and was advised this is quite acceptable. While it may not be your personal preference it does not mean it was inaccurate let alone uncommon in Ruby. Your answer never worked and did not pass the cops so it could not be accepted. — Chris Hough, Aug 11 '16 at 19:26

Ruby method refactor multi line assignment to pass RuboCop

1 Answers1