This week's book giveaway is in the OCPJP forum.
We're giving away four copies of OCA/OCP Java SE 7 Programmer I & II Study Guide and have Kathy Sierra & Bert Bates on-line!
See this thread for details.
The moose likes Performance and the fly likes Reducing Cyclomatic complexity for this code Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of OCA/OCP Java SE 7 Programmer I & II Study Guide this week in the OCPJP forum!
JavaRanch » Java Forums » Java » Performance
Bookmark "Reducing Cyclomatic complexity for this code" Watch "Reducing Cyclomatic complexity for this code" New topic
Author

Reducing Cyclomatic complexity for this code

surya panda
Greenhorn

Joined: Apr 18, 2013
Posts: 1
Hi Can any one suggest how to consolidate the multiple if checks and reducing cyclomatic complexity of below piece of code?
TIA
Surya

if (chkLst.get(0).equalsIgnoreCase("NO")) {
do1();
}

if (chkLst.get(1).equalsIgnoreCase("NO")) {
do2();
}

if (chkLst.get(2).equalsIgnoreCase("NO")) {
do3();
}

if (chkLst.get(3).equalsIgnoreCase("NO")) {
do4();
}

if (chkLst.get(4).equalsIgnoreCase("NO")) {
do5();
}

if (chkLst.get(5).equalsIgnoreCase("NO")) {
do6();
}

if (chkLst.get(6).equalsIgnoreCase("NO")) {
do7();
}

Deepak Bala
Bartender

Joined: Feb 24, 2006
Posts: 6662
    
    5

Can you provide some more context to the question ? What is `chkLst` ? A List<String> ? What do those methods do ?


SCJP 6 articles - SCJP 5/6 mock exams - More SCJP Mocks
Carles Gasques
Ranch Hand

Joined: Apr 19, 2013
Posts: 199
    
    1
You could use reflection?

public void doit(int idx, List<String> chkLst) throws Exception {
if("NO".equalsIgnoreCase(chkLst.get(idx))) {
Method method = YourMethodHolder.class.getMethod("do"+idx);
method.invoke(obj);
}
}



surya panda wrote:Hi Can any one suggest how to consolidate the multiple if checks and reducing cyclomatic complexity of below piece of code?
TIA
Surya

if (chkLst.get(0).equalsIgnoreCase("NO")) {
do1();
}

if (chkLst.get(1).equalsIgnoreCase("NO")) {
do2();
}

if (chkLst.get(2).equalsIgnoreCase("NO")) {
do3();
}

if (chkLst.get(3).equalsIgnoreCase("NO")) {
do4();
}

if (chkLst.get(4).equalsIgnoreCase("NO")) {
do5();
}

if (chkLst.get(5).equalsIgnoreCase("NO")) {
do6();
}

if (chkLst.get(6).equalsIgnoreCase("NO")) {
do7();
}

Jeff Verdegan
Bartender

Joined: Jan 03, 2004
Posts: 6109
    
    6

I wouldn't advise reflection, but you could do something like this:



Of course, without more real-world details about what these list items are and what your "do" methods actually, er ... do, it's impossible to say if there's not some better, simpler overall approach you could be taking.


Carles Gasques
Ranch Hand

Joined: Apr 19, 2013
Posts: 199
    
    1
Hi Jeff,

I'm not sure that changing the if statements for a switch reduces the cyclomatic complexity of a method.

Reflection don't avoid this complexity either, the independent paths still are there, but the code is more compact and it hide the complexity to the inspection code tools.

Cheers,

Jeff Verdegan
Bartender

Joined: Jan 03, 2004
Posts: 6109
    
    6

Carles Gasques wrote:Hi Jeff,

I'm not sure that changing the if statements for a switch reduces the cyclomatic complexity of a method.


I'm not sure either, but the loop + method + switch approach leads to the equivalent of an if/else-if situation, rather than a bunch of independent ifs. Not sure if that will make a difference, but it might. And if it doesn't, the map instead of the switch might. There are ultimately going to be some minimum number of decision points, but breaking them out into separate methods should reduce the complexity of any one method.

Mainly though I'm just tossing out suggestions for the OP to ponder and play with and adapt.

Winston Gutkowski
Bartender

Joined: Mar 17, 2011
Posts: 8046
    
  22

Jeff Verdegan wrote:Mainly though I'm just tossing out suggestions for the OP to ponder and play with and adapt...

@surya: And here are a couple more for you:

1. If each of your 'chkLst' entries can only have two values (presumably "YES" and "NO"), consider a BitSet.

2. If each of your "do" methods has the same signature and the number of possibilities is finite (both of which look likely from what you've shown us) , define them in an enum (Option?) and use an EnumSet. The nice thing about this approach is:
(a) It's practically self-documenting (if you choose your names well).
(b) You don't need to make any decisions at all. Running your optional code becomes simply:
Like Jeff (I suspect), I go a long way to avoid reflection if I can; it's really only needed when you can't know what you need to do until runtime.

Winston

PS: I find it a bit counter-intuitive that you run your options when chkLst.get(n) equals "NO" - unless you run something else when it's "YES".
 
It is sorta covered in the JavaRanch Style Guide.
 
subject: Reducing Cyclomatic complexity for this code