CODEWITHSUNDEEP
python
Travis
Travis

Reputation: 55

Python: Roman Numeral to Int, hitching on 'C' calculation

My code isn't printing the correct number for one of the test cases - "MCMXCVI"

It should print: 1996. It's currently printing 2106.

I suspect it's failing on the validation of the 'C' element after checking an 'M' element in my last elif statement. However, I typed it correctly and don't know why it would be. Any help would be appreciated!

Edit: So I still can't figure it out. I know it's failing on "IV" and "MCM" - but why? I'm parsing everything in and it SHOULDN'T - I'm definitely missing something. I need some n00b explaining, please!

2nd Edit: Got it! I traced "IV" METICULOUSLY and super slowly by hand - I figured out just as I added the V (5), I'd minus 1 AFTERWARDS, instead of adding 5 - 1 (4). So I'd end up with 5. This logic is failing on the others as well. I'll have to first check the value of the element before, THEN add. Thanks all!

class Solution:
def romanToInt(self, roman):
    """
    :type s: str
    :rtype: int
    """
    sum = 0
    for element in range(0, len(roman)):
        if roman[element] == 'I':
            sum += 1

        elif roman[element] == 'V':
            sum += 5
            if roman[element - 1] == 'I':
                sum -= 1

        elif roman[element] == 'X':
            sum += 10
            if roman[element - 1] == 'I':
                sum -= 1

        elif roman[element] == 'C':
            sum += 100
            if roman[element - 1] =='X':
                sum -= 10

        elif roman[element] == 'L':
            sum += 50
            if roman[element - 1] == 'X':
                sum -= 10

        elif roman[element] == 'D':
            sum += 500
            if roman[element - 1] == 'C':
                sum -= 100

        elif roman[element] == 'M':
            sum += 1000
            if roman[element - 1] == 'C':
                sum -= 100

    return sum

Upvotes: 0

Views: 83

Answers (1)

Eypros
Eypros

Reputation: 5723

Since there is not universal acceptance of how the Roman numbers should be used. E.g. from wikipedia for example both MCMX and MDCCCCX represent 1910 you should check for every minor number of each digit not just the previous one as you do.

The remarks I could make is:

  • You should include both I and V subtraction in X for example. And for the other bigger digits also include all previous smaller numbers. That is for L add X, V and I etc.
  • If you keep your code as it is you must subtract twice the value since you already have added it once (in the previous step that is). E.g. IV will be evaluated as +1,+5,-1=+5 not +4. You need +1,+5,-2=+5
  • You should check if you are on the first digit i.e. element=0 then element-1 has different meaning (last digit that is) and you don't want to check it out.

Maybe your code needs more bug fixing but that's what I found.

Upvotes: 1

Related Questions