Convert an integer to its Roman numeral equivalent

Question

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[1]) as data:
 for line in data.readlines():
 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?

Answer 1

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)

• 301_Moved_Permanently: Alternatively, count, number = divmod(number, value)
• papasmurf: @MathiasEttinger What is divmod()?
• 301_Moved_Permanently: @papasmurf help(divmod)

Answer 2

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"

• Graipher: if not (0 < number) is a lot harder to understand than if number <= 0.