# Convert an integer to its Roman numeral equivalent

I got sent a challenge from a friend;

You are given a file that contains numbers, one number per line. Convert the numbers from the file into roman numerals.

Test input:
45
65
23
98
123
7754

Expected Output:
XLV
LXV
XXIII
XCVIII
CXXIII
MMMMMMMDCCLIV


I think I've succeeded in doing this successfully, and would like some critque on what I've done so far:

import sys

ROMAN_NUMERAL_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)
]

def convert_to_roman(number):
""" Convert an integer to Roman
>>> print(convert_to_roman(45))
XLV """
roman_numerals = []
for numeral, value in ROMAN_NUMERAL_TABLE:
while value <= number:
number -= value
roman_numerals.append(numeral)

return ''.join(roman_numerals)

if __name__ == '__main__':
with open(sys.argv) as data:
print("{} to roman numerals: {}".format(line.rstrip(), convert_to_roman(int(line.rstrip()))))


Key points that I would like to focus on (obviously feel free to critique the entire program)

1. Using the tuples for the list, is this good practice, or would a dict have worked better?
2. While converting the integers to the roman numerals is there a easier way to do this, right now my program will take a long time, for an integer that is higher than 12345679990 is there a way to speed up the processing for extremely high integers?

1. Using the tuples for the list, is this good practice, or would a dict have worked better?

Yes, in this case I think it's ok. Alternative is using orderdict or dict but sorting it by value which is not that good.

Now to make it work faster you can use division and multiplication. So it will look like this:

def convert_to_roman(number):
""" Convert an integer to Roman
>>> print(convert_to_roman(45))
XLV """
roman_numerals = []
for numeral, value in ROMAN_NUMERAL_TABLE:
count = number // value
number -= count * value
roman_numerals.append(numeral * count)

return ''.join(roman_numerals)


Using the tuples for the list, is this good practice, or would a dict have worked better?

I would have just used a tuple instead of a list to enforce immutability.

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


You should also check if the number you are converting is valid.

class ToRomanError(Exception): pass
class OutOfRangeError(ToRomanError): pass
class NotIntegerError(ToRomanError): pass

def convert_to_roman(number):
if not (0 < number):
raise OutOfRangeError, "number must be non-negative"
if int(number) != number
raise NotIntegerError, "cannot convert decimals"

• if not (0 < number) is a lot harder to understand than if number <= 0. – Graipher Nov 22 '16 at 6:42