How to reduce 'complexity too high' with ||

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

I've got a simple method which counts total lesson's hours in university schedule for additional modules in the department (student can attend to 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

asked Nov 22 '18 at 8:54

eudaimonia

189111

1

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 9: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 9:39

sports_hours - other methods or query objects grouped by :department

– eudaimonia
Nov 22 '18 at 9:48

@MarcinKołodziej thx that good idea!

– eudaimonia
Nov 22 '18 at 10:06

add a comment |

I've got a simple method which counts total lesson's hours in university schedule for additional modules in the department (student can attend to 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

asked Nov 22 '18 at 8:54

eudaimonia

189111

1

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 9: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 9:39

sports_hours - other methods or query objects grouped by :department

– eudaimonia
Nov 22 '18 at 9:48

@MarcinKołodziej thx that good idea!

– eudaimonia
Nov 22 '18 at 10:06

add a comment |

I've got a simple method which counts total lesson's hours in university schedule for additional modules in the department (student can attend to 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

asked Nov 22 '18 at 8:54

eudaimonia

189111

I've got a simple method which counts total lesson's hours in university schedule for additional modules in the department (student can attend to 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

ruby-on-rails cyclomatic-complexity

asked Nov 22 '18 at 8:54

eudaimonia

189111

asked Nov 22 '18 at 8:54

eudaimonia

189111

asked Nov 22 '18 at 8:54

eudaimonia

189111

asked Nov 22 '18 at 8:54

eudaimonia

189111

asked Nov 22 '18 at 8:54

eudaimonia

189111

1

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 9: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 9:39

sports_hours - other methods or query objects grouped by :department

– eudaimonia
Nov 22 '18 at 9:48

@MarcinKołodziej thx that good idea!

– eudaimonia
Nov 22 '18 at 10:06

add a comment |

1

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 9: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 9:39

sports_hours - other methods or query objects grouped by :department

– eudaimonia
Nov 22 '18 at 9:48

@MarcinKołodziej thx that good idea!

– eudaimonia
Nov 22 '18 at 10:06

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 9: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 9:39

sports_hours - other methods or query objects grouped by :department

– eudaimonia
Nov 22 '18 at 9:48

@MarcinKołodziej thx that good idea!

– eudaimonia
Nov 22 '18 at 10:06

add a comment |

1 Answer
1

active

oldest

votes

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 Nov 22 '18 at 11:04

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

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

add a comment |

Your Answer

StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");

StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "1"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});

function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: true,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: 10,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});

}
});

draft saved

draft discarded

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53427075%2fhow-to-reduce-complexity-too-high-with-or-operator%23new-answer', 'question_page');
}
);

Post as a guest

Name

Required, but never shown

1 Answer
1

active

oldest

votes

1 Answer
1

active

oldest

votes

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 Nov 22 '18 at 11:04

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

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

add a comment |

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 Nov 22 '18 at 11:04

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

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

add a comment |

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 Nov 22 '18 at 11:04

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

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 Nov 22 '18 at 11:04

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

edited Nov 22 '18 at 11:04

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

answered Nov 22 '18 at 10:31

Marcin Kołodziej

4,3111315

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

add a comment |

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

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

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

add a comment |

draft saved

draft discarded

Thanks for contributing an answer to Stack Overflow!

Please be sure to answer the question. Provide details and share your research!

But avoid …

Asking for help, clarification, or responding to other answers.

Making statements based on opinion; back them up with references or personal experience.

To learn more, see our tips on writing great answers.

draft saved

draft discarded

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Post as a guest

Name

Required, but never shown

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Sign up or log in

StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});

Post as a guest

Name

Required, but never shown

Name

Required, but never shown

Name

Required, but never shown

This page is only for reference, If you need detailed information, please check here

搜尋此網誌

Ytukyg