Converting Roman numerals to integers and vice versa

2018-02-02 21:28:02

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

def int_to_roman (integer):

returnstring=''

table=[['M',1000],['CM',900],['D',500],['CD',400],['C

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

    continueye

    2018-02-02 21:48:40
  • 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 u

    2018-02-02 21:53:53
  • def find_roman_to_num(string): # string form roman input

    romans={'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}

    if romans.get(string):

    return romans.get(string)

    else:

    number=0

    idx=1000

    lx=['IIII','XXXXX','IXI','CDC','CMC','LL','DD','CCCC','VV']

    for i in lx:

    if i in string:

    print i,string

    return None

    while string:

    first=string[0]

    second=string[:2]

    if romans.get(first) and not romans.get(second):

    number += romans.get(first)

    string = string[1:]

    n_idx=romans.get(first)

    print n_idx,idx

    if n_idx > idx:

    return None

    idx=n_i

    2018-02-02 22:06:16