How to reduce 'complexity too high' with || - or operator

Question

I've got a simple method that counts total lesson hours in the university schedule for additional modules in the department (students can attend many departments)

 def hours_total
    @hours_total = user.open_departments.each_with_object({}) do |department, h|
      h[department] = (sports_hours[department] || 0) +
                    (science_hours[department] || 0) +
                    (intership_sum[department] || 0) +
                    (art[department] || 0) -
                    ((obligatory_topics[department] || 0) +
                    (base[department] || 0))
    end
  end

How can I fix here Cyclomatic complexity for hours_total is too high.? I have no idea how to not repeat || 0 cause in some departments sports_hours[department] can be nil value

This code doesn't make sense. Where do all these variables, like `sports_hours`, come from? A good start would be to separate each of those conditionals into different methods. — gd_silva, Nov 22 '18 at 09:31
In your case, I'd try `.to_i` instead of `|| 0`, you should refactor it further though. — Marcin Kołodziej, Nov 22 '18 at 09:39
sports_hours - other methods or query objects grouped by :department — eudaimonia, Nov 22 '18 at 09:48

score 1 · Accepted Answer · edited Sep 28 '22 at 15:34

1

The first step I'd take

def hours_total
  @hours_total = user.open_departments.each_with_object({}) do |department, h|
    positive = [sport_hours, science_hours, internship_sum, art].sum do |pos_h|
                 pos_h[department].to_i
               end

    negative = [obligatory_topics, base].sum do |neg_h|
                 neg_h[department].to_i
               end

    h[department] = positive - negative
  end
end

Note: if your hours can be float values, substitute to_i with to_f.

Now if you and your Rubocop are ok with that, I'd probably leave it. If any of you is unhappy, the positive and negative should be extracted to a method.

edited Sep 28 '22 at 15:34

Jai Chauhan

4,035
3
36
62

answered Nov 22 '18 at 10:31

Marcin Kołodziej

5,253
1
10
17

wouldn't `to_i` lead to an incorrect value? assuming time in hours then it could be `2.2 hours` and typecasting it to `integer` would result in incorrect value there. – Abhinay Nov 22 '18 at 11:00
ideally it should typecast only in case of `nil` value – Abhinay Nov 22 '18 at 11:02
1

Good point, when reading the original code I was under the impression that hours would be integers. I think `to_f` would be more generic, I'll add a note about that. – Marcin Kołodziej Nov 22 '18 at 11:02
@Abhinay I'm not sure whether I'd be afraid of `to_f` or `to_i` here, I'd just make sure I'm using the correct method depending on the input. Might be habit though. – Marcin Kołodziej Nov 22 '18 at 11:06

How to reduce 'complexity too high' with || - or operator

1 Answers1