CODEWITHSUNDEEP
pythonalgorithmroman-numerals
Mahmoud Hanafy
Mahmoud Hanafy

Reputation: 8228

Converting roman numerals to integers

I've been given a task to convert roman numerals to integers and have been able to come up with the following solution:

def roman_numeral_to_int(string):
    symbols = {
        'I': 1,
        'V': 5,
        'X': 10,
        'L': 50,
        'C': 100,
        'D': 500,
        'M': 1000
    }
    repetitions = {}
    result = 0
    skip = 0
    for i, c in enumerate(string):
        if i == skip and i != 0:
            continue
        if c not in symbols:
            return None
        if c in repetitions.items():
            repetitions[c] += 1
        else:
            repetitions = {c: 1}
        for r, v in repetitions.items():
            if (r in ['L', 'D', 'V'] and v > 1) or (r in ['I', 'X', 'C'] and v > 3):
                return None
        if c == 'I':
            # last character in the string
            if i == len(string) - 1:
                result += 1
            elif string[i+1] == 'V':
                result += 4
                skip = i + 1
            elif string[i+1] == 'X':
                result += 9
                skip = i + 1
        elif c == 'X':
            # last character in the string
            if i == len(string) - 1:
                result += 10
            elif string[i+1] == 'L':
                result += 40
                skip = i + 1
            elif string[i+1] == 'C':
                result += 90
                skip = i + 1
        elif c == 'C':
            # last character in the string
            if i == len(string) - 1:
                result += 100
            elif string[i+1] == 'D':
                result += 400
                skip = i + 1
            elif string[i+1] == 'M':
                result += 900
                skip = i + 1
        else:
            skip = 0
            result += symbols[c]
    return result

However, this solution gets the wrong answer with the string MLXVI which should output 1066, while this code yields 1056.

Can someone please point out what's wrong with this solution?

Upvotes: 0

Views: 1341

Answers (2)

Yann Vernier
Yann Vernier

Reputation: 15887

Just quickly reading, but...

if i == skip and i != 0 wouldn't need a special case for the first characted if skip wasn't precisely 0.

if c in repetitions.items() seems exceedingly unlikely to work; the items are tuples and c is a character.

repetitions should probably be a collections.Counter.

You've written special cases for a bunch of characters (IXC) even though the rule is consistent: lower magnitude before higher magnitude is subtracted. Among other things, these special cases have hardcoded values.

Those special cases include an implicit case where characters are ignored.

Upvotes: 1

damores
damores

Reputation: 2351

That specific case would be fixed by adding an else statement in this block:

elif c == 'X':
    # last character in the string
    if i == len(string) - 1:
        result += 10
    elif string[i+1] == 'L':
        result += 40
        skip = i + 1
    elif string[i+1] == 'C':
        result += 90
        skip = i + 1
    else:
        result += 10

However I see that this may be happening in other places like with multiple Is. So III should be 3 but your program returns 1.

Upvotes: 0

Related Questions