Waz
Waz

Reputation: 161

Refactoring java method to reduce cyclomatic complexity

    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.

Upvotes: 2

Views: 7607

Answers (1)

NESPowerGlove
NESPowerGlove

Reputation: 5496

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

Upvotes: 2

Related Questions