HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Roman numeral kata in Elixir

Submitted by: @import:stackexchange-codereview··
0
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
end

Solution

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 = decimal do
    _convert(number - decimal, roman_value ++ roman, numerals)
  end

  defp _convert(number, roman_value, [_ | numerals]) do
    _convert(number, roman_value, numerals)
  end
end

Code Snippets

def converts(number, roman_value, [[decimal, roman] | numerals]) when number >= decimal do
  converts(number - decimal, roman_value ++ roman, [[decimal, roman] | numerals])
end
def converts(number, roman_value, [[decimal, roman] | _] = numerals) 
  when number >= decimal do
  converts(number - decimal, roman_value ++ roman, numerals)
end
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

Context

StackExchange Code Review Q#27310, answer score: 7

Revisions (0)

No revisions yet.