Roman Numeral Conversion in Java I have designed this by using the stack data structure (similar to balanced parentheses problem) checking if the previous element is decreasing the number or not.

package test;

import main.algorithms.RomanConversion;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;

public class RomanConversionTest {

    RomanConversion romanConversion;

    @Before
    public void setUp(){
        romanConversion = new RomanConversion();
    }

    @Test
    public void testRomanConversion(){
        Assert.assertEquals(154, romanConversion.convert("CLIV"));
        Assert.assertEquals(101, romanConversion.convert("CI"));
        Assert.assertEquals(43, romanConversion.convert("XLIII"));
    }

}




package main.algorithms;

import java.util.HashMap;
import java.util.Stack;

public class RomanConversion {

    public HashMap createMap(){
        HashMap&lt;String, Integer&gt; romanMap = new HashMap&lt;&gt;();
        romanMap.put("I" , 1);
        romanMap.put("IV",4);
        romanMap.put("V" ,5);
        romanMap.put("IX",9);
        romanMap.put("X" ,10);
        romanMap.put("XL",40);
        romanMap.put("L" ,50);
        romanMap.put("XC",90);
        romanMap.put("C" ,100);
        romanMap.put("CD",400);
        romanMap.put("D" ,500);
        romanMap.put("CM",900);
        romanMap.put("M" ,1000);

        return romanMap;
    }


    public int convert(String letter) {

        int result=0;
        HashMap&lt;String, Integer&gt; map = new HashMap&lt;&gt;();
        map = createMap();
        String str;
        char[] array = letter.toCharArray();
        Stack&lt;Character&gt; stack = new Stack&lt;&gt;();

        for(char c: array){
            if(stack.isEmpty()){
                result += map.get(String.valueOf(c));
                stack.push(c);
            }else{
                char lastElement= stack.peek();
                if(map.containsKey(lastElement+""+c)) {
                    result -= map.get(String.valueOf(lastElement));
                    result += map.get(String.valueOf(lastElement) + String.valueOf(c));
                    stack.push(c);
                }else{
                    result += map.get(String.valueOf(c));
                    stack.push(c);
                }
            }

        }
        return result;
    }
}
I think your code is - in general - good. Your algorithm is working correctly and I really like the idea of using a Hashmap to save the values of the roman numbers.



One thing I miss is the handling of edge cases. Especially you should think about the following edge cases:


letter == null
Empty String
Strings that aren't valid roman numbers (for example "IXI" or "MMMM" aren't valid)


The first two problems are pretty easy to solve:

if(letter == null || letter.equals("")) {
        return -1;
}


The third part is a bit more difficult to solve:

if(!letter.matches("M{0,3}(CM|CD|D?C{0,3})(XC|XL|L?X{0,3})(IX|IV|V?I{0,3})$")) {
        return -1;
}


As you can see, you can use a regular expression to validate if the string letter contains a valid roman number (as explained for example here).



One thought about your testing-class: You are using JUnit, which is good, but I think you are testing much too little (remember the edge cases!). 

I wrote a test myself with the following values:

String[] input = {null, "", "I", "V", "XXXIII", "DCCXLVII", "CMXXIX", "MCCXXXII", "MMMCMXCIX", "MMMMXI", "KMXI", "VX", "VL", "VC", "VD", "VM","LC", "LD", "LM", "DM", "IL", "IC", "ID", "IM", "XD", "XM", "CXLV", "MIXI", "IXI", "MXIII", "MMMM", "IIII"}; 

int[] expectedOutput = {-1, -1, 1, 5, 33, 747, 929, 1232, 3999, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 145, -1, -1, 1013, -1, -1};