Refactoring java method to reduce cyclomatic complexity

Question

    ArrayList cars = vehicles.getExtras();

    Iterator it = cars.iterator();
    while(it.hasNext()){
        Extra cExtra = (Extra)it.next();

            if (cExtra.getExtraId()==INSURANCE)
                insurance = cExtra.getExtra();

            if (cExtra.getExtraId()==ROAD_ASSISTANCE_CODE)
                roadAssCode = cExtra.getExtra();

            if (cExtra.getExtraId()==TYPE)
                replacementCarServiceType = cExtra.getExtra();

            if (cExtra.getExtraId()==CATEGORY)
                replacementCarCategory = cExtra.getExtra();

            if (cExtra.getExtraId()==ASSISTANCE){
                roadAssistanceCode = cExtra.getExtra();
            }

I have a lot more of these 'if' types that i have not included. How could i refactor this, so the cyclomatic complexity could be reduced.

But, if you do use the `if`s, please use `else if` when the subsequent cases cannot occur if the prior conditions are true. — Andy Thomas, Feb 16 '15 at 19:53
If you used a Visitor pattern, you could extract all the ugliness into the visitor itself. — Hovercraft Full Of Eels, Feb 16 '15 at 19:54
Switch statement will still not reduce the cyclomatic complexity. Raw list is used as the code was written a long time ago — Waz, Feb 16 '15 at 19:57
Hovercraft Full Of Eels, how would i go about this for my method. Can you give me a general direction? — Waz, Feb 16 '15 at 20:02
This doesn't look like the correct duplicate at all. Waz wants to know a way to reduce CC stats/overuse of switch/if-elses while the duplicate marked is a question about other ways that languages handle switch statements. — NESPowerGlove, Feb 16 '15 at 20:28
@NESPowerGlove, thanks. I checked that answer and it wasnt what i was looking for. Do you have any idea's as to how i can achieve this? — Waz, Feb 16 '15 at 21:33
Check [this link](http://stackoverflow.com/questions/4469123/how-to-improve-cyclomatic-complexity). I would again recommend either the Visitor Pattern or the Command Pattern, the latter of which could use a `Map`. You could look up how to use either of these constructs. — Hovercraft Full Of Eels, Feb 16 '15 at 21:48

score 2 · Accepted Answer · edited Jun 20 '20 at 09:12

Polymorphism is usually the key to reducing CC. If you have a group of similar things, that do similar but different things, then perhaps those things can inherit from a common base class but override a certain method to provide distinct behavior.

Sometimes you may hear people proclaim that you can use polymorphism to remove all ifs, however there's usually a fine balance between the two extremes. You usually want to strive first to meet the SOLID principles and with that lower CC will come.

The book Refactoring: Improving the Design of Existing Code is a pretty good read all across this subject. Apparently Refactoring to Patterns is too but I have not read it.

Here's an example of how one could refactor your code to get a cyclomatic complexity of 1 (I don't know exactly how your code is setup but I think I recreated the idea of it):

import java.util.ArrayList;
import java.util.List;

public class ExtraTest {
    public static void main(String[] whatsUp) {
        MyData myData = new MyData();

        List<Extra> myExtras = new ArrayList<>();
        myExtras.add(new ExtraInsurance("ReallyBadInsurance"));
        myExtras.add(new ExtraCarCategory(CarCategory.really_big_truck));

        System.out.println("Data before: " + myData);

        myExtras.forEach(extra -> extra.applyExtra(myData));

        System.out.println("Data after: " + myData);
    }

    public static enum CarCategory {not_a_truck, truck, big_truck, really_big_truck}

    public static class MyData {
        String insurance = "none";
        CarCategory carCategory = CarCategory.not_a_truck;

        @Override
        public String toString() {
            return insurance + " : " + carCategory.toString();
        }
    }

    public abstract static class Extra<T> {
        protected final T extraAttributeToProvide;

        public Extra(T extraAttributeToProvide) {
            this.extraAttributeToProvide = extraAttributeToProvide;
        }

        public abstract void applyExtra(MyData myData);
    }

    public static class ExtraInsurance extends Extra<String> {
        public ExtraInsurance(String extraAttributeToProvide) {
            super(extraAttributeToProvide);
        }

        public void applyExtra(MyData myData) {
            myData.insurance = extraAttributeToProvide;
        }
    }

    public static class ExtraCarCategory extends Extra<CarCategory> {

        public ExtraCarCategory(CarCategory extraAttributeToProvide) {
            super(extraAttributeToProvide);
        }

        public void applyExtra(MyData myData) {
            myData.carCategory = extraAttributeToProvide;
        }
    }

}

Outputs:

Data before: none : not_a_truck

Data after: ReallyBadInsurance : really_big_truck

Refactoring java method to reduce cyclomatic complexity

1 Answers1