Преобразование из/римские цифры


Я писал в качестве упражнения в Разработка через тестирование кусок кода на Python, который содержит две функции:

  • roman2dec(роман), который преобразует римские цифры (строки) в десятичное число (int)
  • dec2roman(декабря), которая преобразует десятичное число (int) в римское число (строка)

Я хотел бы прочитать ваши комментарии на этот код, если есть плохой практики, если ты не расписывался за доставку или если вы не хотите ничего менять.

import re
import math

# Regular expression used to validate and parse Roman numbers
roman_re = re.compile("""^
   ([M]{0,9})   # thousands
   ([DCM]*)     # hundreds
   ([XLC]*)     # tens
   ([IVX]*)     # units
   $""", re.VERBOSE)

# This array contains valid groups of digits and encodes their values.
# The first row is for units, the second for tens and the third for
# hundreds. For example, the sixth element of the tens row yields the
# value 50, as the first is 0.
d2r_table = [
    ['', 'I', 'II', 'III', 'IV', 'V', 'VI', 'VII', 'VIII', 'IX'],
    ['', 'X', 'XX', 'XXX', 'XL', 'L', 'LX', 'LXX', 'LXXX', 'XC'],
    ['', 'C', 'CC', 'CCC', 'CD', 'D', 'DC', 'DCC', 'DCCC', 'CM']]


def roman2dec(roman):
    """Converts a roman number, encoded in a string, to a decimal number."""
    roman = roman.upper()
    match = roman_re.match(roman)

    if not match:
        raise ValueError

    thousands, hundreds, tens, units = match.groups()
    result = 1000 * len(thousands)
    result += d2r_table[2].index(hundreds) * 100
    result += d2r_table[1].index(tens) * 10
    result += d2r_table[0].index(units)

    return result


def dec2roman(dec):
"""Converts a positive decimal integer to a roman number."""
    if dec == 0:
        return ''

    digit = 0
    rem = dec
    result = ''

    # Length in digits of the number dec
    dec_len = int(math.ceil(math.log10(dec)) + 1)

    # Scan the number digit-by-digit, starting from the MSD (most-significant
    # digit)
    while dec_len > 0:
        # Let's take the current digit
        factor = 10 ** (dec_len - 1)
        digit = rem / factor

        # And remove it from the number
        rem = rem - digit * factor

        if dec_len >= 4:
            # Thousands
            result = result + digit * 'M'
        else:
            # Look in the look-up table
            result = result + d2r_table[dec_len - 1][digit]

        dec_len -= 1

    return result

Редактировать: вот тестов для dec2roman:

class DecToRoman(unittest.TestCase):
    def testZeroIsEmpty(self):
        self.checkString(0, "")

    def testSingleDigits(self):
        self.checkString(1, "I")
        self.checkString(10, "X")
        self.checkString(50, "L")
        self.checkString(100, "C")
        self.checkString(500, "D")
        self.checkString(1000, "M")

    def testSimpleRepeats(self):
        self.checkString(1, "I")
        self.checkString(2, "II")
        self.checkString(3, "III")
        self.checkString(10, "X")
        self.checkString(20, "XX")
        self.checkString(30, "XXX")

    def testSubtraction(self):
        self.checkString(4, "IV")
        self.checkString(9, "IX")
        self.checkString(40, "XL")
        self.checkString(90, "XC")

    def testOther(self):
        self.checkString(89, "LXXXIX")
        self.checkString(145, "CXLV")
        self.checkString(691, "DCXCI")
        self.checkString(1983, "MCMLXXXIII")
        self.checkString(2412, "MMCDXII")
        self.checkString(3309, "MMMCCCIX")

    def checkString(self, decimal, expected_string):
        self.assertEqual(expected_string, new_dec2roman(decimal))


6396
10
задан 21 февраля 2011 в 04:02 Источник Поделиться
Комментарии
1 ответ

Прежде всего вы должны документировать то, что ваш код не работает с числами выше 9999.

Тогда я думаю, что ваш код станет немного проще, если вы добавляете четвертую строку в ваш d2r_table для тысяч. Чтобы избежать повторения вы можете использовать список понимание:

d2r_table = [
['', 'I', 'II', 'III', 'IV', 'V', 'VI', 'VII', 'VIII', 'IX'],
['', 'X', 'XX', 'XXX', 'XL', 'L', 'LX', 'LXX', 'LXXX', 'XC'],
['', 'C', 'CC', 'CCC', 'CD', 'D', 'DC', 'DCC', 'DCCC', 'CM'],
['M' * i for i in xrange(0,10) ]]

Это позволяет не рассматривать как особый случай. Поэтому вместо того, чтобы:

thousands, hundreds, tens, units = match.groups()
result = 1000 * len(thousands)
result += d2r_table[2].index(hundreds) * 100
result += d2r_table[1].index(tens) * 10
result += d2r_table[0].index(units)

вы можете написать:

thousands, hundreds, tens, units = match.groups()
result = d2r_table[3].index(thousands) * 1000
result += d2r_table[2].index(hundreds) * 100
result += d2r_table[1].index(tens) * 10
result += d2r_table[0].index(units)
return result

Теперь вы можете легко заметить общую закономерность: для каждого числа от 0 до 3, Что ты принимаешь меняой строке d2r_table, называя индекса на ней с 3-яго элемента из группы в качестве аргумента, умножив его в 10**я и, наконец, подводя итоги. Вы можете реферат общий шаблон, как это, если вы хотите:

def value_for_group(i):
group = match.groups[3-i]
return d2r_table[i].index(group) * 10**i

return sum(value_for_group(i) for i in xrange(0,4))

Вы также можете заменить магические числа 3 и 4, с чем-то более значимым:

num_rows = len(d2r_table)

def value_for_group(i):
group = match.groups[num_rows - 1 - i]
return d2r_table[i].index(group) * 10**i

return sum(value_for_group(i) for i in xrange(0, num_rows))

Таким образом, ваш код будет работать без изменений, если вы когда-нибудь изменить d2r_table с учетом расширенные римские цифры.


В вашем dec2roman функции вы используете целочисленной арифметики для перебора цифр числа. Я думаю, было бы проще и яснее, чтобы просто преобразовать число в строку и перебирать цифры с помощью цикла for. В обратном d2r_table, вы можете использовать молнии , чтобы выполнить итерации по таблице и цифры параллельно без какого-либо индекса на основе петли. Таким образом, ваш dec2roman функция будет выглядеть следующим образом:

result = ''
# Make digits four digits long so it has the same number of digits
# as d2r_table has rows, then convert each digit to an int
digits = [int(digit) for digit in "%04d" % dec]

for digit, d2r_row in zip(digits, reversed(d2r_table)):
result += d2r_row[ digit ]

return result

Вы можете также использовать объединить с генератор выражение вместо обновления результат повелительно:

digits = [int(digit) for digit in "%04d" % dec]
table = reversed(d2r_table)
return ''.join( d2r_row[ digit ] for digit, d2r_row in zip(digits, table) )

Опять-таки возможно, вы захотите заменить 4 в %04д с лен(d2r_table), так что ваш код будет автоматически адаптироваться к более d2r_table.

11
ответ дан 21 февраля 2011 в 06:02 Источник Поделиться