Roman numerals solution #2
Conversation
| final String numStr = numArray[i]; | ||
| boolean e = true; | ||
| for (int k = 0; k < numStr.length(); k++) { | ||
| if (Character.isDigit(numStr.charAt(k)) == false) { |
There was a problem hiding this comment.
You don't need the == false, use if (! condition)
|
|
||
| } | ||
|
|
||
| public static void readInput() { |
There was a problem hiding this comment.
This should be private, and this applies to the other methods.
| } | ||
| } | ||
|
|
||
| if (e) { |
There was a problem hiding this comment.
it is hard to figure out what you are testing here.
| } | ||
|
|
||
| public static void convert() { | ||
| for (int i = 0; i < numArray.length; i++) { |
There was a problem hiding this comment.
In general, you should use collections, because there are way better functions to work with them than with arrays.
|
|
||
| } | ||
|
|
||
| public static void runTest() { |
There was a problem hiding this comment.
Would be better to write a JUnit test
| runTest(); | ||
| } | ||
| } else { | ||
| readInput(); |
There was a problem hiding this comment.
I like this style, but the methods should return something and the next method should use it as input.
| public final static String TWELVE = "XII"; | ||
| public final static String ONE_HUNDRED_AND_THREE = "CIII"; | ||
|
|
||
| private static String[] numArray; |
There was a problem hiding this comment.
You either solve this problem in an OOP fashion or functional, but this is approach is none of them. You use a static variable to pass around value. If you create two RomanNumeralsConvert this won't work.
| line = line.replace("\"", "").trim(); | ||
| numArray = line.split("\\s+"); | ||
| } | ||
| } catch (final IOException e) { |
There was a problem hiding this comment.
If you catch the exception here, the validate() method will still run
| public final static String tensArray[] = { "X", "XX", "XXX", "XL", "L", "LX", "LXX", "LXXX", "XC" }; | ||
| public final static String hundredsArray[] = { "C", "CC", "CCC", "CD", "D", "DC", "DCC", "DCCC", "CM" }; | ||
|
|
||
| public final static String TEST_MOD = "-test"; |
There was a problem hiding this comment.
Move test data somewhere else.
| if (args.length > 1 || !TEST_MOD.equals(args[0])) { | ||
| System.out.println("Only one flag supports: - test"); | ||
| } else { | ||
| runTest(); |
There was a problem hiding this comment.
See later, but it would be better to have tests in a JUnit class.
| } | ||
|
|
||
| public static void validate() { | ||
| for (int i = 0; i < numArray.length; i++) { |
|
|
||
| final int ones = num % 10; | ||
|
|
||
| num = (num - ones) / 10; |
There was a problem hiding this comment.
Can you make this and the following steps a bit more simpler?
|
|
||
| num = (num - hundreds) / 10; | ||
| for (int n = 0; n < num; n++) { | ||
| Roman += "M"; |
There was a problem hiding this comment.
You have "M" here, but the rest is at the top.
using collection change convert algorithm exit if catch exception changed methods access modifiers fix condition in if statement
Hi, my solution for roman-numerals task.