Your code looks very nice indeed, I only have some minor observations you code use to improve this code:
Naming
Use the imperative form of the verb (convert
instead of converts
)
Implementation Hiding
Use defp
for methods which are not intended for external use. You might consider also renaming them to start with underscore _
to further differentiate them as internal.
Multiple assignment in method signature
To make you code more succinct, you can use multiple assignment - meaning that instead of:
def converts(number, roman_value, [[decimal, roman] | numerals]) when number >= decimal do
converts(number - decimal, roman_value ++ roman, [[decimal, roman] | numerals])
end
you can write:
def converts(number, roman_value, [[decimal, roman] | _] = numerals)
when number >= decimal do
converts(number - decimal, roman_value ++ roman, numerals)
end
to the same effect. This form assigns the first item in the list to [decimal, roman]
, as well as assigning the whole list (including the first element) to numerals
, so you don't have to re-build it inside the method body.
Implementing the above suggestions, your code will look like this:
defmodule RomanNumeral do
@decimal_roman_numerals [[5, 'V'], [4, 'IV'], [1, 'I']]
def convert(number) do
_convert(number, [], @decimal_roman_numerals)
end
defp _convert(number, roman_value, _) when number < 1 do
roman_value
end
defp _convert(number, roman_value, [[decimal, roman] | _] = numerals)
when number >= decimal do
_convert(number - decimal, roman_value ++ roman, numerals)
end
defp _convert(number, roman_value, [_ | numerals]) do
_convert(number, roman_value, numerals)
end
end