'Python - Convert a roman numeral to an integer

I've tried the following Python 3 code that transform from Roman to Integer.

The code is working fine at a glance. But there are certain problem happened when I input X, V or related to X and V value.

For example: when I try V the code is not working but for IV showing correct value.

My code:

class Solution(object):
   def romanToInt(self, s):
      """
      :type s: str
      :rtype: int
      """
      roman = {'I':1,'V':5,'X':10,'L':50,'C':100,'D':500,'M':1000,'IV':4,'IX':9,'XL':40,'XC':90,'CD':400,'CM':900}
      i = 0
      num = 0
      while i < len(s):
         if i+1<len(s) and s[i:i+2] in roman:
            num+=roman[s[i:i+2]]
            i+=2
         else:
            #print(i)
            num+=roman[s[i]]
            i+=1
      return num
ob1 = Solution()

message = str(input("Please enter your roman number: "))
if message <= ("MMMCMXCIX"):
   print (ob1.romanToInt(message))
else:
    print ("Try again")

I've set the condition which is, if the input roman number is equal or less than MMMCMXCIX, it will print the roman number; else it will print Try again.

The problem is when I input X, V or related to X and V value the output is showing Try again

Please help me to understand where I went wrong.



Solution 1:[1]

That is because you are comparing the strings instead first convert the roman to int than check

class Solution(object):
   def romanToInt(self, s):
      """
      :type s: str
      :rtype: int
      """
      roman = {'I':1,'V':5,'X':10,'L':50,'C':100,'D':500,'M':1000,'IV':4,'IX':9,'XL':40,'XC':90,'CD':400,'CM':900}
      i = 0
      num = 0
      while i < len(s):
         if i+1<len(s) and s[i:i+2] in roman:
            num+=roman[s[i:i+2]]
            i+=2
         else:
            #print(i)
            num+=roman[s[i]]
            i+=1
      return num
ob1 = Solution()

message = str(input("Please enter your roman number: "))
# converting to integer than comparing the value of "MMMCMXCIX"
if ob1.romanToInt(message) <= 8999:
   print (ob1.romanToInt(message))
else:
    print ("Try again")

Solution 2:[2]

In the end its just an addition of numbers, you just need to figure if they need to be interpreted positive or negativ:

roman = {'I':1,'V':5,'X':10,'L':50,'C':100,'D':500,'M':1000,'IV':4,'IX':9,'XL':40,'XC':90,'CD':400,'CM':900}

def roman2Dec(inp):
    inpNum = [roman[x] for x in inp]
    return sum([-x if i < len(inpNum)-1 and x < inpNum[i+1] else x for i, x in enumerate(inpNum)])

for nums in [('IX', 9), ('XL', 40), ('LXI', 61), ('MMMCMXCIX', 3999)]:
    result = roman2Dec(nums[0])
    print result == nums[1]

Output:

True
True
True
True

Solution 3:[3]

As mentioned in the comments, the problem was caused because you compared string rather than first transform to int and then do the comparison.

Other approach that you can try is:

def romanToInt(s):
  d = {'m': 1000, 'd': 500, 'c': 100, 'l': 50, 'x': 10, 'v': 5, 'i': 1}
  n = [d[i] for i in s.lower() if i in d]
  return sum([i if i>=n[min(j+1, len(n)-1)] else -i for j,i in enumerate(n)])

print(romanToInt('X')) # 10
print(romanToInt('V')) # 5
print(romanToInt('IV')) # 4
print(romanToInt('XV')) # 15

In my opinion both are more pythonic.

Solution 4:[4]

Listed below some approaches below with their respective runtimes to convert roman numerals to integers.

Solution 1: (Approx Runtime = 52ms)

def romanToInt(self, s: str) -> int:
 s = s.upper()
 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:
 s = s.upper()
 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:
 s = s.upper()
 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 :)

Solution 5:[5]

You can solve this question in a simple way as below.

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

Solution 6:[6]

def romanToInt(s):    
    p=z=0
    for i in map('IVXLCDM'.find,s[::-1]):x=[1,5][i%2]*10**(i//2);z+=[x,-x][p>x];p=x
    return z

Solution 7:[7]

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

def roman2Dec(s):
    res = 0
    num_list = [roman[char] for char in s]
    print(num_list)
    length = len(num_list)
    for i in range(length):
        if i < length-1 and num_list[i] < num_list[i+1]:
            res -= num_list[i]
        else:
            res += num_list[i]
    return res
 
s = "MCMXCIV"
print(roman2Dec(s)) # output: 1994

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1 DaVinci
Solution 2 Maurice Meyer
Solution 3
Solution 4 R.K
Solution 5
Solution 6 Chanchhunneng chrea
Solution 7 Amr Khalil