I want to write a function to check if a given string in roman presentation is correct or not. They are a lot of cases of non allowed combinations : (I assume that the given string will represent a number between 1 and 3999)
- We can't have the same character more than three times in a row : ex : IIII is false.
- Some combinations are not allowed : DD is false ('D' + 'D' = 500 + 500 = 1000 which is M
- We can't substract a character and add the same character just after : for example IXI is not correct event if IX is 9, it's not equal to 9 + 1
- The most significant digits has to be at the beginning not at the middle or at the end. Ex : XM (for 1010) is false while MX is the correct one.
- etc...
So, my idea was that rather than checking for the non allowed combinations, I will write ALL the possible allowed combinations and each time we meet a combination which is not among them, we will return false. That was my idea. The inconvenient is that my final function was very long and not really easy to understand.
For example, I wrote first a function to check for the thousands (if they exist of course), the function then returns the indexes that I will use to substring the current string to move to the next part (which will be hundreds in that case) :
private static int isThousandsValid(String str){
int len = str.length();
char a1 = str.charAt(0);
char a2 = (len >= 2)? str.charAt(1) : ' ';
char a3 = (len >= 3)? str.charAt(2) : ' ';
if (a1 == 'M' && a2 == 'M' && a3 == 'M') //if we met that combinatin
return 3; //we have to move after 3 digits to meet the beginning
//of the hundred digits
else if (a1 == 'M' && a2 == 'M') //same raisoning for other combinations
return 2;
else if (a1 == 'M')
return 1;
else if (a1 == 'D' || a1 == 'C' || a1 == 'L' || a1 == 'X' || a1 == 'V' || a1 == 'I' )
return 0;
else return -1;
}
Then, I wrote the same thing for hundreds, tens and units. Example for hundreds :
private static int isHundredsValid(String str){
if (str.isEmpty()) return 0;
int len = str.length();
char a1 = str.charAt(0);
char a2 = (len >= 2)? str.charAt(1) : ' ';
char a3 = (len >= 3)? str.charAt(2) : ' ';
char a4 = (len >= 4)? str.charAt(3) : ' ';
if (a1 == 'C' && a2 == 'M')
return 2;
else if (a1 == 'D' && a2 == 'C' && a3 == 'C' && a4 == 'C')
return 4;
else if (a1 == 'D' && a2 == 'C' && a3 == 'C')
return 3;
else if (a1 == 'D' && a2 == 'C')
return 2;
else if (a1 == 'D')
return 1;
else if (a1 == 'C' && a2 == 'D')
return 2;
else if (a1 == 'C' && a2 == 'C' && a3 == 'C')
return 3;
else if (a1 == 'C' && a2 == 'C')
return 2;
else if (a1 == 'C')
return 1;
else if (a1 == 'L' || a1 == 'X' || a1 == 'V' || a1 == 'I' )
return 0;
else return -1;
}
Then, in my final function, I write this :
public static boolean isValidRoman(String str){
str = str.trim(); //remove spaces
if (str.isEmpty()) return false;
int index1 = isThousandsValid(str);
String str1 = mySubstring(str, index1);
int index2 = isHundredsValid(str1);
String str2 = mySubstring(str1, index2);
int index3 = isTensValid(str2);
String str3 = mySubstring(str2, index3);
int index4 = isUnitsValid(str3);
String str4 = mySubstring(str3, index4);
if (str1.isEmpty() || str2.isEmpty() || str3.isEmpty())
return true;
if (index1 == -1 || index2 ==-1 || index3 == -1 || index4 == -1)
return false;
return str4.isEmpty(); //if we still have ANOTHER character after it terminates
}
Finally "mySubstring" is just a simple function that I used to refactor and clear my code :
private static String mySubstring(String str, int index){
if (index == -1 ) return str;
else
return str.substring(index);
}
I have please two main questions : Does this function seem correct for you? I had tested in many examples but I'm not really sure (I can't test all the 3999 possible combinations...)
Is it possible to improve it? Just to make it cleaner or more readable? Is there any easier way to check for the validity of roman number rather than write all those cases??