Rubocop: Assignment Branch Condition size for hashes

[firedev]

Say I need to create a big hash for API response. This is an example:

def hello
{
  a: a,
  b: b,
  c: c,
  d: d,
  e: e,
  f: f,
  g: g,
  h: h,
  j: j,
  i: i,
  k: k,
  l: l,
  m: m,
  n: n,
  o: o,
  p: p,
  q: q,
  r: r
}
end

Rubocop complaints that assignment branch condition is too high, but there is no actual assignment (apart from hash keys) and certainly no branches or conditions.

What one is supposed to do in this case?

[jonas054]

"Branch" in the ABC metric doesn't mean conditional route through the code, it means a function call. So it's all the calls to the method b that generate the high number. I'm guessing this is a made-up example, not your real code, so it's difficult to come up with good advice how to decrease the ABC size.

[firedev]

Okay, here what actual code looks like:

def prepare_purchase_params(options)
  {
    sum: receipt.sum_with_discount,
    cashbox: cashbox,
    receipt_uuid: receipt.uuid,
    purchase_positions_attributes: prepare_receipt_positions(receipt),
    pharmacy_uuid: Pharmacy.current_uuid,
    is_return: options[:receipt_uuid].present?,
    card_number: options[:card_id].presence,
    certificate: options[:certificate].presence,
    pin_code: options[:pin_code].presence
  }
end

[firedev]

After splitting it to a few methods Rubocop accepts it but the code doesn't look much better. Maybe even more convoluted.

[jonas054]

Perhaps you're looking at the problem too narrowly if you just try to refactor the method so that it passes inspection. It might be a good time to look at the data design. Why is the hash so big? Should all these values really be properties of a purchase? There's a reference to a receipt, but there's also a sum, which can be retrieved from the receipt. Should card_number and pin_code be moved to a new CreditCard type? I don't know the context, so I'm just asking.

If, in the end, you decide that the design is fine as it is, then a local disabling with # rubocop:disable Metrics/AbcSize could be the way to go. This _is_ a large method in ABC terms, but perhaps it's exceptional in that way and your other methods can be inspected with a low threshold for this metric.

[firedev]

Thanks, was wondering what the general consensus here

[jennwah]

Perhaps you're looking at the problem too narrowly if you just try to refactor the method so that it passes inspection. It might be a good time to look at the data design. Why is the hash so big? Should all these values really be properties of a purchase? There's a reference to a receipt, but there's also a sum, which can be retrieved from the receipt. Should card_number and pin_code be moved to a new CreditCard type? I don't know the context, so I'm just asking.

If, in the end, you decide that the design is fine as it is, then a local disabling with # rubocop:disable Metrics/AbcSize could be the way to go. This _is_ a large method in ABC terms, but perhaps it's exceptional in that way and your other methods can be inspected with a low threshold for this metric.

yo this is life saver haha

[marcandre]

Better comment on #8986.
Note that the example given passes with default settings.