False positive in "Methods should not contain too many return statements"

[Michał Kordas]

Hi,

I have feedback for rule "Methods should not contain too many return statements" (squid:S1142).

The following code triggers violation Reduce the number of returns of this method 4, down to the maximum allowed 3:

int enumToNumberConverter() {

    switch(number) {

        case ZERO: return 0;

        case ONE: return 1;

        case TWO: return 2;

        case THREE: return 3;

    }

}

I don't think that if there are returns just in switch statement there is any brain-overload. There would be overload if there were addtional returns outside switch.

Regards,

Michal Kordas

[Michel Pawlak]

Well, I don't agree with you. Three exit points is already a lot. Even if you have lots of preconditions to check it's a lot : you should use JSR303 to lower this number (one of the "rare" cases where you can have more is in the equals() method).

Back to your code, if it is a real example, it would be better to refactor your Enum's code in order to get something similar to :

public enum NumberEnum {
  ONE(1),
  TWO(2),
  THREE(3),
  FOUR(4);

  private int value;

  public NumberEnum(int value) {
    this.value = value;
  }

  public int toInt() {
    return this.value;
  }
}

Then you can just call the toInt() method and get rid of all these switch cases.

Kind regards

[Michał Kordas]

Hi Michel,

Sorry for the late reply. I've just oversimplified my example. In our system we have e.g. enums that represent external systems that we connect to.

public enum UpstreamSystem {
  SYSTEM_1,
  SYSTEM_2,

  SYSTEM_3,

  SYSTEM_4;
}

Then we have hundreds of places where we need to distinguish between these four - general flow is same for every system, but details differ. Putting entire logic to this enum would make it God Class, what we want to aviod :)

Please consider my request again.

Thanks,

Michal

[Michel Pawlak]

Hi Michal,

Then again, IMHO the problem may not be on the rule side, but on the conception side. Have a look at Strategy, Template method or Specification behavioural design patterns. They may help you in your context. OOP, design patterns and polymorphism provides you with powerful means to avoid multiplying condition checks and return statements.

Cheers,