I have an method to convert roman numbers to common decimal. I used here an one for cycle and a lot of "if" conditions. SonarLint in my IDE says me that Cognitive Complexity of this method is 33 while 15 is allowed. How can I reduce this? I do not mind how can I fix this. Waiting for your reccomendations!
public static int roman2Decimal(String roman) {
int decimal = 0;
char previous = 0;
for (int x = 0; x < roman.length(); x++) {
if (roman.charAt(x) == 'I')
decimal += 1;
if (roman.charAt(x) == 'V') {
System.out.println(previous);
if (previous == 'I') {
decimal -= 2;
}
decimal += 5;
}
if (roman.charAt(x) == 'X') {
if (previous == 'I') {
decimal -= 2;
}
decimal += 10;
}
if (roman.charAt(x) == 'L') {
if (previous == 'X') {
decimal -= 20;
}
decimal += 50;
}
if (roman.charAt(x) == 'C') {
if (previous == 'X') {
decimal -= 20;
}
decimal += 100;
}
if (roman.charAt(x) == 'D') {
if (previous == 'C') {
decimal -= 200;
}
decimal += 500;
}
if (roman.charAt(x) == 'M') {
if (previous == 'C') {
decimal -= 200;
}
decimal += 1000;
}
previous = roman.charAt(x);
}
return decimal;
}
The first step would be replacing the repeated if
statements with a switch
:
public static int roman2Decimal(String roman) {
int decimal = 0;
char previous = 0;
for (int x = 0; x < roman.length(); x++) {
switch (roman.charAt(x)) {
case 'I':
decimal += 1;
break;
case 'V':
if (previous == 'I') {
decimal -= 2;
}
decimal += 5;
break;
case 'X':
if (previous == 'I') {
decimal -= 2;
}
decimal += 10;
break;
case 'L':
if (previous == 'X') {
decimal -= 20;
}
decimal += 50;
break;
case 'C':
if (previous == 'X') {
decimal -= 20;
}
decimal += 100;
break;
case 'D':
if (previous == 'C') {
decimal -= 200;
}
decimal += 500;
break;
case 'M':
if (previous == 'C') {
decimal -= 200;
}
decimal += 1000;
break;
}
previous = roman.charAt(x);
}
return decimal;
}
If we go further with refactoring, we might notice other repeated patterns. Using an enum would help making this method much more concise:
enum RomanDigit {
ZERO(0, null), // sentinel
I(1, ZERO),
V(5, I),
X(10, I),
L(50, X),
C(100, X),
D(500, C),
M(1000, C);
public final int inc;
public final RomanDigit prev;
RomanDigit(int inc, RomanDigit prev) {
this.inc = inc;
this.prev = prev;
}
}
public static int roman2Decima2l(String roman) {
int decimal = 0;
RomanDigit previous = RomanDigit.ZERO;
for (char c : roman.toCharArray()) {
RomanDigit current = RomanDigit.valueOf(String.valueOf(c));
if (previous.equals(current.prev)) {
decimal -= 2 * previous.inc;
}
decimal += current.inc;
previous = current;
}
return decimal;
}