Converting from Roman numerals to Arabic numbers

I got an idea to make a Roman/Arabic number converter, and I would like to hear your thoughts, where I could improve it, if there are Python specific things that I missed, so on and so forth.

# Decodes roman numerals
def get_arabic_numbers(t):
# Hold the numbers in the list
l = list()

# The format
format = 0

# Convert from roman numeral to arabic number
for i in range(0, len(t)):
if t[i] == 'I':
l.append(int(1))
elif t[i] == 'V':
l.append(int(5))
elif t[i] == 'X':
l.append(int(10))
elif t[i] == 'L':
l.append(int(50))
elif t[i] == 'C':
l.append(int(100))
elif t[i] == 'D':
l.append(int(500))
elif t[i] == 'M':
l.append(int(1000))

# Calculate the format as follows:
i = 0
while i < len(l) - 1:
# If the next numeral is greater than the current one, add the next one minus the current one = format = format + next_numeral - current_numeral
if l[i] < l[i+1]:
format = format + l[i+1] - l[i];
i = i + 2
else:
format = format + l[i]
i = i + 1

# Fix in case the last two numerals are equal
if i == len(l):
if l[i-2] == l[i-1]:
format = format + l[i-1]
else:
format = format + l[i]

# Return the format
return format

# Encode an arabic digit to a roman numeral
def encode(l, c, one, five, nine):
# The format to be returned
format = ""

# Special case 5.1: 5 has its own numeral 'V'
# Special case 5.2: 50 has its own numeral 'A'
# Special case 5.3: 500 has its own numeral 'D'
if l[c] == 5:
format = format + five
elif l[c] < 5:
for i in range(0, l[c]):
format = format + one
elif l[c] > 5:
# Special case 9.1: 9 depends on 10, so it's 'IX'
# Special case 9.2: 90 depends on 1000, so it's 'XC'
# Special case 9.3: 900 depends on 1000, so it's 'CM'
if l[c] == 9:
format = format + nine
else:
format = format + five
for i in range(6, l[c] + 1):
format = format + one

return format

# Encodes arabic numbers
def get_roman_numerals(t):
# The arabic number to be converted
t = int(t)
# The list contains the number's digits
l = list()

# int is not iterable, so I wrote this to get the digits
while t >= 1:
l.append(int(t % 10))
t = t / 10

# They are in the wrong order, so the list has to be reversed
l.reverse()

# The format
format = ""

# Current position in the number
c = 0

# If the number is at least 1000
if len(l) >= 4:
num = 0

# All the digits after 1000
i = int(len(l) - 1)
while i > 3:
num = num + 1
i = i - 1

# A list containing these digits
li = list()

for i in range(0, num + 1):
li.append(l[i])

# Converts the list to an int
n = map(str, li)
n = ''.join(n)
n = int(n)

# Add an 'M' for every 1000
for i in range(0, n):
format = format + 'M'

c = num + 1

# If the number is also least 100
if len(l) >= 3:
format = format + encode(l, c, 'C', 'D', "CM")
c = c + 1

# If the number is at least 10
if len(l) >= 3:
format = format + encode(l, c, 'X', 'L', "XC")
c = c + 1

# If the number is at least 100 but smaller than 1000
if len(l) > 2:
format = format + encode(l, c, 'I', 'V', "IX")

# If the number is at least 10 but smaller than 100
if len(l) == 2:
format = format + encode(l, c, 'X', 'L', "XC")
c = c + 1
format = format + encode(l, c, 'I', 'V', "IX")

# If the number is at least 1 but smaller than 10
if len(l) == 1:
format = format + encode(l, c, 'I', 'V', "IX")

return format
• Fun to see. I wrote a Roman numeral calculator when learning Fortran some 40+ years ago. Commented Sep 15, 2016 at 13:01
• I just started reading about Python a few hours ago, and this seemed the easiest example to see how to use Python, that I could think of. Commented Sep 15, 2016 at 14:29

Impressions

That's a very long solution. The comments were generally helpful, but the code itself could have been easier to understand if it were shorter and more expressive.

The variable names were generally not helpful: l, i, t, c, li were all cryptic.

get_arabic_numbers(t) is not really named appropriately:

• "get" implies retrieving something that already exists. This is more of a calculation.
• Why is "numbers" plural?
• Why is the parameter named t — what does it stand for?
• Strictly speaking, the result is an int. It's not Arabic, nor is it even in base ten. It's just an abstract integer, which is conventionally rendered for you as base-ten Arabic digits when str() is called on it (whether explicitly or implicitly). (And by "Arabic", we mean 0123456789, not ٠١٢٣٤٥٦٧٨٩.)

Based on these concerns, I'd call it decode_roman_numeral(roman). It happens to be what you wrote as the comment! Similarly, I'd call the inverse function encode_roman_numeral(num).

Arabic → Roman

# The format
format = 0

That's not what I would call a "format" — I would call it a "result".

# Hold the numbers in the list
l = list()
…

# Convert from roman numeral to arabic number
for i in range(0, len(t)):
if t[i] == 'I':
l.append(int(1))
elif t[i] == 'V':
l.append(int(5))
elif t[i] == 'X':
l.append(int(10))
elif t[i] == 'L':
l.append(int(50))
elif t[i] == 'C':
l.append(int(100))
elif t[i] == 'D':
l.append(int(500))
elif t[i] == 'M':
l.append(int(1000))

As I mentioned before, l is a cryptic name. There is no need to convert 1 to int(1). Lookups are typically done in Python using a dictionary. Also, whenever you populate a list by creating an empty list and appending to it repeatedly, that is a good candidate for a list comprehension:

trans = {'I': 1, 'V': 5, 'X': 10, 'L': 50, 'C': 100, 'D': 500, 'M': 1000}
values = [trans[r] for r in roman]

# Calculate the format as follows:
i = 0
while i < len(l) - 1:
# If the next numeral is greater than the current one, add the next one minus the current one = format = format + next_numeral - current_numeral
if l[i] < l[i+1]:
format = format + l[i+1] - l[i];
i = i + 2
else:
format = format + l[i]
i = i + 1

result = 0
for i in range(0, len(l) - 1):
if l[i] < l[i+1]:
result -= l[i]
else:
result += l[i]

As I mentioned before, the code could be more expressive. This loop is summing values in a list, so we should use the sum() built-in function to say what we mean.

result = sum(
val if val >= next_val else -val
for val, next_val in zip(values[:-1], values[1:])
)

All together, then, I'd condense your function down to this:

def decode_roman_numeral(roman):
"""Calculate the numeric value of a Roman numeral (in capital letters)"""
trans = {'I': 1, 'V': 5, 'X': 10, 'L': 50, 'C': 100, 'D': 500, 'M': 1000}
values = [trans[r] for r in roman]
return sum(
val if val >= next_val else -val
for val, next_val in zip(values[:-1], values[1:])
) + values[-1]

Arabic → Roman

The encode() function encodes a single Arabic digit, so encode_digit() would be a better name. There is no point in passing l and c separately, since you only ever care about l[c]. Instead of looping to append characters to format, use the * operator to construct a repeated string.

I would write this helper function using a single expression with four cases:

def encode_digit(digit, one, five, nine):
return (
nine                     if digit == 9 else
five + one * (digit - 5) if digit >= 5 else
one + five               if digit == 4 else
one * digit
)

The get_roman_numerals() function is rather convoluted. I had a hard time following the code, so I went by your comments instead. In the end, I figured out that it was essentially doing this:

def encode_roman_numeral(num):
num = int(num)
return (
'M' * (num // 1000) +
encode_digit((num // 100) % 10, 'C', 'D', 'CM') +
encode_digit((num //  10) % 10, 'X', 'L', 'XC') +
encode_digit( num         % 10, 'I', 'V', 'IX')
)
• Woah, result -= l[i] is very clever and makes the code so much better, congratulations! Commented Sep 15, 2016 at 7:19
• I should probably mention two things: 1. I wrote this from a C perspective, 2. I wrote this after 7 hours of reading about Python, I have never used it before. get_arabic_numbers was supposed to be get_arabic_digits, I copied the name for get_roman_numerals, and I forgot it was supposed to be digits. The sum function seems interesting, also the return in encode_digit, also what is this // ? Never saw it before, in C you use it for comments. Commented Sep 15, 2016 at 14:27
• How are these possible: values[:-1], values[1:]) and values[-1] ? And what does :-1 and 1: represent? Commented Sep 15, 2016 at 14:48
• The // operator performs integer division. Commented Sep 15, 2016 at 18:19
• Negative indexes count backwards from the end — values[-1] is the same as values[len(values) - 1]. The : is a slice operatorvalues[:-1] makes a copy of values except the last element, and values[1:] is a copy of everything after the first element. Commented Sep 15, 2016 at 18:23

I'll focus on get_arabic_numbers. First, a few local issues:

# Hold the numbers in the list
l = list()

You can come up with a better name than l which tells us nothing about what's in the list.

# The format
format = 0

You call it format which makes it look like it's a string, but it's actually a number. Also, "The format" adds no value as a comment. Either find something more descriptive that cannot be inferred by looking at the code or remove the comment.

# Convert from roman numeral to arabic number
for i in range(0, len(t)):

Simply use for c in t. It's very rare to use index-based loops in Python because most of the time you can simply iterate over the elements of your collection.

if t[i] == 'I':
l.append(int(1))

Why int(1)? 1 is enough.

elif t[i] == 'V':
l.append(int(5))

This is quite repetitive, and you can use a dictionary here. Simply declare it as a constant like this:

ROMAN_TO_INT = {'I': 1, 'V': 5, 'X': 10, 'L': 50, 'C': 100, 'D': 500, , 'M': 1000}

Now you can rewrite this whole loop like this:

for c in t:
l.append(ROMAN_TO_INT[c])

You even get an exception if the letter is not in your dictionary, while your previous code silently ignored the issue (you had no else case). Note that you could transform this into a generator function, like this:

def convert_individual_letters(roman_string):
for c in roman_string:
yield ROMAN_TO_INT[c]

There are three benefits of doing this:

1. You don't need a name for l
2. It's a generator that can directly be used in a for loop (it can even more efficient if the rest of your code use generators too because you won't iterate over the whole string unless you asked for it - however it's not the case here)
3. You get to name your function, which improves clarity

And you can use this function in two ways. Either simply l = list(convert_individual_letters('MDC')) to consume the whole generator or with a for loop: for n in convert_individual_letters('MDC').

Okay, so now you converted something like MDCCCLXXIX to [1000, 500, 100, 100, 100, 50, 10, 10, 1, 10] and the goal is to get 1879.

# Calculate the format as follows:
i = 0
while i < len(l) - 1:

In this case, it looks like you do need an index, so a while loop is appropriate.

# If the next numeral is greater than the current one, add the next one minus the current one = format = format + next_numeral - current_numeral
if l[i] < l[i+1]:
format = format + l[i+1] - l[i];
i = i + 2
else:
format = format + l[i]
i = i + 1

Great, makes sense. Use format += l[i] instead of format = format + l[i]. One possible issue: If I send you "IC", you'll happily say 99. Is this valid?

# Fix in case the last two numerals are equal
if i == len(l):
if l[i-2] == l[i-1]:
format = format + l[i-1]
else:
format = format + l[i]

Hmm, don't you think there's a better way than having to fix this case? One option is to add similar numbers together in convert_individual_letters, an another one is to change the loop:

i = 0
while i < len(l):
# If the next numeral is greater than the current one, add the next one minus the current one = format = format + next_numeral - current_numeral
if i < len(l) - 1 and l[i] < l[i+1]:
format = format + l[i+1] - l[i];
i = i + 2