patternMinor
Roman numeral kata in Elixir
Viewed 0 times
katanumeralromanelixir
Problem
I'm new to Elixir, and in order to learn the syntax, I'm doing a roman numeral kata which converts a decimal number into roman numeral. I would appreciate any feedback you have.
defmodule RomanNumeral do
@decimal_roman_numerals [[5, 'V'], [4, 'IV'], [1, 'I']]
def converts(number) do
converts(number, [], @decimal_roman_numerals)
end
def converts(number, roman_value, _) when number = decimal do
converts(number - decimal, roman_value ++ roman, [[decimal, roman] | numerals])
end
def converts(number, roman_value, [_ | numerals]) do
converts(number, roman_value, numerals)
end
endSolution
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 (
Implementation Hiding
Use
Multiple assignment in method signature
To make you code more succinct, you can use multiple assignment - meaning that instead of:
you can write:
to the same effect. This form assigns the first item in the list to
Implementing the above suggestions, your code will look like this:
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])
endyou can write:
def converts(number, roman_value, [[decimal, roman] | _] = numerals)
when number >= decimal do
converts(number - decimal, roman_value ++ roman, numerals)
endto 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 = decimal do
_convert(number - decimal, roman_value ++ roman, numerals)
end
defp _convert(number, roman_value, [_ | numerals]) do
_convert(number, roman_value, numerals)
end
endCode Snippets
def converts(number, roman_value, [[decimal, roman] | numerals]) when number >= decimal do
converts(number - decimal, roman_value ++ roman, [[decimal, roman] | numerals])
enddef converts(number, roman_value, [[decimal, roman] | _] = numerals)
when number >= decimal do
converts(number - decimal, roman_value ++ roman, numerals)
enddefmodule 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
endContext
StackExchange Code Review Q#27310, answer score: 7
Revisions (0)
No revisions yet.