# Converting Roman numerals to integers and vice versa

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>=0:

integer-=pair
returnstring+=pair

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):

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

else: continueyes=False
else: continueyes=False

return returnint


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>=0:


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

            integer-=pair
returnstring+=pair


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):

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


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
string=string[len(pair):]

else: continueyes=False
else: continueyes=False

return returnint


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):]
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.

• thank you! This is incredibly helpful. This all makes a lot of sense. I learned python (my first prog. language since turbo pascal in 1980s) very quickly and with specific tasks in mind, really don't yet know how to utilize its features... Oct 3 '11 at 23:09

Looks good. I have a few thoughts, every one of them very minor and based in opinion.

• I think it would be clearer to iterate over roms and nums instead of having pairs and having to remember which is which. This uses a Python feature called 'tuple unpacking':

for (rom, num) in table:
print rom
print num

• Concatenating strings over and over is slower than appending to a list - but that this is something that would likely never matter for this application! If you want, you could collect your Roman numerals in a list before joining them at the end:

l = []
for i in range(10):
l.append('s')
s = "".join(l)
print s

• table is information common to both functions; not that it's going to change, but if evidence for new Roman numerals ever was found, it'd be nice to just add them in one place. table could therefore be a module-level variable.

• I personally find continueyes to be an awkward variable name - you could use continue_, following a convention of adding a trailing underscore to avoid a Python keyword.

• You could use break instead of setting continueyet = True and waiting for the while to check the condition.

while True:
if done:
break