patternpythonMinor

Converting Roman numerals to integers and vice versa

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

viceromanversanumeralsandconvertingintegers

Problem

def int_to_roman (integer):

    returnstring=''
    table=[['M',1000],['CM',900],['D',500],['CD',400],['C',100],['XC',90],['L',50],['XL',40],['X',10],['IX',9],['V',5],['IV',4],['I',1]]

    for pair in table:

        while integer-pair[1]>=0:

            integer-=pair[1]
            returnstring+=pair[0]

    return returnstring

def rom_to_int(string):

    table=[['M',1000],['CM',900],['D',500],['CD',400],['C',100],['XC',90],['L',50],['XL',40],['X',10],['IX',9],['V',5],['IV',4],['I',1]]
    returnint=0
    for pair in table:

        continueyes=True

        while continueyes:
            if len(string)>=len(pair[0]):

                if string[0:len(pair[0])]==pair[0]:
                    returnint+=pair[1]
                    string=string[len(pair[0]):]

                else: continueyes=False
            else: continueyes=False

    return returnint

Solution

def int_to_roman (integer):

    returnstring=''
    table=[['M',1000],['CM',900],['D',500],['CD',400],['C',100],['XC',90],['L',50],['XL',40],['X',10],['IX',9],['V',5],['IV',4],['I',1]]

The element in your list should really be tuples not lists. It should also be a global constant so that you can reuse across both functions.

for pair in table:

Use for letter, value in table: rather then indexing the tuples.

while integer-pair[1]>=0:

I think the code looks better with spacing around binary operators. Also why this instead of: while integer >= pair[1]:?

integer-=pair[1]
            returnstring+=pair[0]

It'll probably be better to create and append to list and then join the list elements together at the end.

return returnstring

def rom_to_int(string):

    table=[['M',1000],['CM',900],['D',500],['CD',400],['C',100],['XC',90],['L',50],['XL',40],['X',10],['IX',9],['V',5],['IV',4],['I',1]]
    returnint=0
    for pair in table:

        continueyes=True

Whenever I use a logical flag like this, I think: it should be removed. I figure that flags like this only serve to confuse the logic of what you are doing. i think a break is clearer then setting a flag.

while continueyes:
            if len(string)>=len(pair[0]):

                if string[0:len(pair[0])]==pair[0]:

strings have a funciton: startswith that does this. You should use it here. There is also need to check the length. If you take a slice past the end of a string in python, you just get a shorter string.

returnint+=pair[1]
                    string=string[len(pair[0]):]

                else: continueyes=False
            else: continueyes=False

    return returnint

My version of your code:

def int_to_roman (integer):
    parts = []
    for letter, value in TABLE:
        while value <= integer:
            integer -= value
            parts.append(letter)
    return ''.join(parts)

def rom_to_int(string):
    result = 0
    for letter, value in table:
        while string.startswith(letter):
            result += value
            string = string[len(pairs[0]):]
    return result

One last thought. Your rom_to_int doesn't handle the case where an invalid string is passed. You might want to consider having it throw an exception or something in that case.

Code Snippets

def int_to_roman (integer):

    returnstring=''
    table=[['M',1000],['CM',900],['D',500],['CD',400],['C',100],['XC',90],['L',50],['XL',40],['X',10],['IX',9],['V',5],['IV',4],['I',1]]

for pair in table:

while integer-pair[1]>=0:

integer-=pair[1]
            returnstring+=pair[0]

return returnstring

def rom_to_int(string):

    table=[['M',1000],['CM',900],['D',500],['CD',400],['C',100],['XC',90],['L',50],['XL',40],['X',10],['IX',9],['V',5],['IV',4],['I',1]]
    returnint=0
    for pair in table:


        continueyes=True

Context

StackExchange Code Review Q#5091, answer score: 8

Revisions (0)

No revisions yet.