Roman to Integer python

Question

I am trying to solve this python problem with python3, my code looks like this.

class Solution:
    def romanToInt(self, s: str) -> int:
        # Define integer value to each roman 
        rom_val = {'I': 1, 'V': 5, 'X': 10, 'L': 50,
                  'C': 100, 'D': 500, 'M': 1000}
        # A list of integer values
        value = list(map(rom_val.get, s))
        # The subtracted new number
        new = 0
        # The converted integer number
        integer = 0
        # List to keep the checked roman
        checked = []
        for i, j in enumerate(value):
            if j > value[i+1] or j == value[i+1]:
                checked.append(j)
            if j < value[i+1]:
                new = value[i+1] - j
                checked.append(new)
        return sum(checked)

However, I am getting IndexError: list index out of range on the first if statement. Even though I know this is rather an easy type of question but there is something I don't understand about. So I have two questions: 1. of course, why I am getting this index error? how do I fix it? 2. is my way of solving this problem correct?

Thank you very much.

Answer 1

Here is a different approach, 5 lines:

d = {'M':1000, 'D':500, 'C':100, 'L':50, 'X':10, 'V':5, 'I':1}

def romanToInt(self, s):
    res, p = 0, 'I'
    for c in s[::-1]:
        res, p = res - d[c] if d[c] < d[p] else res + d[c], c
    return res

Basically, going backward, adding each letter to result unless something smaller is in front of something larger in which case a subtraction instead of addition.

---

Note: The following is not a complete solution, just a fix for your error mentioned in the question. There is a major bug in your algorithm.

Like MCMXCIV should be 1994 but it returns 3099. This is because you consider C as 100 and M as 1000 but should be considering CM as 900. Since there is a solution above, I'll leave this as an exercise for you.

Problem with your code is that even if you reach the last index, you check for i + 1. You can fix that like this:

def romanToInt(s: str) -> int:
        # Define integer value to each roman 
        rom_val = {'I': 1, 'V': 5, 'X': 10, 'L': 50,
                  'C': 100, 'D': 500, 'M': 1000}
        # A list of integer values
        value = list(map(rom_val.get, s))

        # List to keep the checked roman
        checked = []
        for i, j in enumerate(value):
            if i == len(value) - 1:
                checked.append(j)
            elif j >= value[i+1]:
                checked.append(j)
            elif j < value[i+1]:
                checked.append(value[i+1] - j)

        print(checked)
        return sum(checked)

print(romanToInt("LVIII"))

I have also made your code a bit more concise, removed unnecessary variables. The major change is only the check if it is the last index on in value.

Answer 2

I know this is an answered post, but I would like to add 3 solutions with their respective runtimes to convert roman numerals to numbers.

Solution 1: (Approx Runtime = 52ms)

def romanToInt(self, s: str) -> int:

 roman = {'I':1, 'V':5, 'X':10, 'L':50, 'C':100, 'D':500, 'M':1000 }    
 num = 0

 for i in range(len(s)):

    if i!= len(s)-1 and roman[s[i]] < roman[s[i+1]]:
         num += roman[s[i]]*-1
    else:
         num += roman[s[i]]

  return num

Solution 2: (Approx Runtime = 60ms)

def romanToInt(self, s: str) -> int:

 roman = {'I':1, 'V':5, 'X':10, 'L':50, 'C':100, 'D':500, 'M':1000 }    
 num = 0

 s = s.replace("IV", "IIII").replace("IX", "VIIII")
 s = s.replace("XL", "XXXX").replace("XC", "LXXXX")
 s = s.replace("CD", "CCCC").replace("CM", "DCCCC")

 for x in s:
    num += roman[x]

 return num

Solution 3: (Approx Runtime = 48ms)

def romanToInt(self, s: str) -> int:

 roman = {'I':1, 'V':5, 'X':10, 'L':50, 'C':100, 'D':500, 'M':1000 }    
 num = 0

 for i in range(len(s)-1):
    if roman[s[i]] < roman[s[i+1]]:
        num += roman[s[i]]*-1
        continue

     num += roman[s[i]]

  num +=roman[s[-1]]

  return num

The simplest solution appears to be the best at times :)

Answer 3

This question can be solved with the idea that every roman numeral in the roman number are arranged in descending order if that's not the case then it is a special case like 'IV', 'IX', 'XL', .....

In the below the above peculiar case is handled by the if statement.

def romanToInt(s):
    mapping  = {
        'I': 1,
        'V': 5,
        'X': 10,
        'L': 50,
        'C': 100,
        'D': 500,
        'M': 1000,
        }
        
    min_ = None
    total = 0
    for c in s:
        val = mapping[c]
        print(val)
        if min_ and val > min_:
            total -= min_*2
        else:
            min_ = val
        total += val
                
    return total

Answer 4

Take a look at this code. It's easy to understand the logic.

Note: The idea is to traverse the roman number in a reverse manner character by character and add if the next character is greater than or equal to the previous one.

def romanToDecimal(self, S): 
    d = {
        'I':1, 'V':5, 'X':10, 'L':50, 'C':100, 'D':500, 'M':1000
    }
    res = 0
    old = 'I'
    
    for i in S[::-1]:
        if d[i] >= d[old]:
            res = res + d[i]
        else:
            res = res - d[i]
        old = i
        
    return res

Answer 5

Here, for example, I pass 'II' to your function than values becomes [1,1]. Now using enumerate you will have (1,1) as i,j in your second iteration which is causing the index error. Then you can solve this easily.