Roman Numerals in Java

I wrote a class that can convert to and fro from Roman Numerals and Decimals. I would appreciate critical points on everything, especially:

• Shortening my code.

• Choosing a better approach.

• Using better/new features of Java 8.

And so on...

I didn't add JavaDoc because I am expecting suggestions for a better approach. I will document it then. Here is the class:

RomanNumeral.java

```
package library.util;

import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;

/**
* A "recreational" class that provides that functionality of converting
* values to and fro from Roman numerals.
*
* @author Subhomoy Haldar
*/
public class RomanNumeral {

private final String representation;
private final int value;

private RomanNumeral(String representation, int value) {
this.representation = representation;
this.value = value;
}

@Override
public String toString() {
return representation;
}

public int getValue() {
return value;
}

public static RomanNumeral of(String representation) throws
NumberFormatException,
ArithmeticException {
// Zero wasn't supported then...
if (representation.isEmpty()) {
throw new NumberFormatException("Empty String.");
}
int value = 0, previous = 0;
StringBuilder builder = new StringBuilder(representation);
builder.reverse(); // go in the opposite manner
for (int i = 0; i = 5_000) {
throw new ArithmeticException("Unsupported value : " + value);
}
int copy = value;
StringBuilder builder = new StringBuilder(10); // 4999 gives 10 chars
// The descending order is maintained
for (Map.Entry entry : map.entrySet()) {
int v = entry.getValue();
String k = entry.getKey();
while (copy >= v) {
copy -= v;
builder.append(k);
}

I like that you use an immutable map. However I don't like the name map: you should give it a more descriptive name and it should be capitalized since it is a constant. You should also declare the type of map as LinkedHashMap: it is important here since you do use the ordering property. You can define map as private static final Map map = new LinkedHashMap() {{ this.put(x, y); ... }}.

Your api should probably return an error on invalid roman numeral instead of: // calling representationOf() makes sure that "IIIII" becomes "V".

In method of (string to int), I think you don't have to use a StringBuilder and to invert it. You can easily modify your algorithm to work forward and just use toCharArray. You could use the Java 8 "functional" (Stream) features here instead of a for-loop. I like functional programming since it makes for shorter and more readable code, but they really messed things up in Java 8 and I couldn't find an easy solution. I could do it in Scala, but in Java 8 it would end up being longer than your code and completely unreadable.

StackExchange Code Review Q#105171, answer score: 5