# What do you think of my solution for converting integers to roman numerals?

What do you think of my solution for converting integers to roman numerals?

``````defmodule RomanNumerals do
import IO, only: [iodata_to_binary: 1]

@doc """
Convert the number to a roman number.
"""
@spec numeral(pos_integer) :: String.t()
def numeral(0),               do: ""
def numeral(1),               do: "I"
def numeral(2),               do: "II"
def numeral(3),               do: "III"
def numeral(4),               do: "IV"
def numeral(5),               do: "V"
def numeral(6),               do: "VI"
def numeral(7),               do: "VII"
def numeral(8),               do: "VIII"
def numeral(9),               do: "IX"
def numeral(10),              do: "X"
def numeral(50),              do: "L"
def numeral(100),             do: "C"
def numeral(500),             do: "D"
def numeral(1000),            do: "M"
def numeral(n) when n < 40,   do: [numeral(10), numeral(n - 10)]                  |> iodata_to_binary()
def numeral(n) when n < 50,   do: [numeral(10), numeral(50), numeral(n - 40)]     |> iodata_to_binary()
def numeral(n) when n < 90,   do: [numeral(50), numeral(n - 50)]                  |> iodata_to_binary()
def numeral(n) when n < 100,  do: [numeral(10), numeral(100), numeral(n - 90)]    |> iodata_to_binary()
def numeral(n) when n < 400,  do: [numeral(100), numeral(n - 100)]                |> iodata_to_binary()
def numeral(n) when n < 500,  do: [numeral(100), numeral(500), numeral(n - 400)]  |> iodata_to_binary()
def numeral(n) when n < 900,  do: [numeral(500), numeral(n - 500)]                |> iodata_to_binary()
def numeral(n) when n < 1000, do: [numeral(100), numeral(1000), numeral(n - 900)] |> iodata_to_binary()
def numeral(n) when n < 5000, do: [numeral(1000), numeral(n - 1000)]              |> iodata_to_binary()
end
``````
6 Likes

Elegant

Nice, but your spec is wrong. It should be non_neg_integer() as You do include 0

4 Likes

Nice catch!

Thanks @kokolegorille ! This is for such comments that I like to post on this forum.

3 Likes

Also, the spec does make the function appear unbound towards positive infinity.

In reality the spec should be `numeral(0..4999) :: String.t()`.

In general, I do consider this appraoch as quite noisy, and also since `iodata_to_binary/1` is used on each recursion step, nothing is gained by it. It might even perform worse than just using `<>/2`.

Last but not least, I’m not a friend of not using `mix format`.

4 Likes

Thanks @NobbZ, I’ll definitely pay more attention to the spec!

Then, I’ll refactor using `<>/2` instead of `iodata_to_binary/1`.

I also generally use `mix format`; but wanted to make an exception. I find it more readable this way.

Also with the way you defined the spec, could it be confused with a range?

How would you write the spec for a function that takes a range of integer from 0 to 4999 then?

This already is how you‘d do that in a typespec: https://hexdocs.pm/elixir/typespecs.html#literals

1 Like

There is `Range.t/2` for that.

1 Like

You could rename your function `def numeral` to `defp do_numeral`, and then define a function `def numeral(n), do: iodata_to_binary(do_numeral(n))` so you have a single call to `iodata_to_binary`.

You could write a macro that by using your function will generate 3999 functions for each number in the range `1-3999` (took the 3999 figure from Wikipedia).

The largest number that can be represented in this notation is 3,999 (3,000 + 900 + 90 + 9 = MMM + CM + XC + IX = MMMCMXCIX )

Macro/Metaprogramming approaches are possible, though my approach usually just takes an ordered list of conversions and iterates over them.

My take inside, it might spoil you!`
``````defmodule Roman do
@arab_to_roman [
{1000, "M"}, {900, "CM"}, {500, "D"}, {400, "CD"}, {100, "C"}, {90, "XC"},
{50, "L"}, {40, "XL"}, {10, "X"}, {9, "IX"}, {5, "V"}, {4, "IV"}, {1, "I"}
]

@doc """
Convert the number to a roman number.
"""
@spec numerals(1..3999) :: String.t
def numerals(number) do
Enum.reduce(@arab_to_roman, {"", number}, fn ({ar, rm}, {acc, n}) ->
{acc <> String.duplicate(rm, div(n, ar)), rem(n, ar)}
end) |> elem(0)
end
end
``````

PS: This was created when `mix format` did not yet exist.

3 Likes

Last idea y @NobbZ is cool, iteration is 100x better than recursion when you have something to iterate on.

My take is still using recursion, but with only one way of handling all cases, because the additional clauses are only adding noise. By checking case from big to small, you have each clause in the same shape.

Click to expand and see my take
``````defmodule Roman do
def numeral(n) when n < 0 or n > 3_999, do: {:error, {:out_of_bounds, n}}
def numeral(n), do: {:ok, do_numeral(n)}

defp do_numeral(n) when n >= 1000, do: "M" <> do_numeral(n - 1000)
defp do_numeral(n) when n >= 900, do: "DM" <> do_numeral(n - 900)
defp do_numeral(n) when n >= 500, do: "D" <> do_numeral(n - 500)
defp do_numeral(n) when n >= 400, do: "CD" <> do_numeral(n - 400)
defp do_numeral(n) when n >= 100, do: "C" <> do_numeral(n - 100)
defp do_numeral(n) when n >= 90, do: "XC" <> do_numeral(n - 90)
defp do_numeral(n) when n >= 50, do: "L" <> do_numeral(n - 50)
defp do_numeral(n) when n >= 40, do: "XL" <> do_numeral(n - 40)
defp do_numeral(n) when n >= 10, do: "X" <> do_numeral(n - 10)
defp do_numeral(n) when n >= 9, do: "IX" <> do_numeral(n - 9)
defp do_numeral(n) when n >= 5, do: "V" <> do_numeral(n - 5)
defp do_numeral(n) when n >= 4, do: "IV" <> do_numeral(n - 4)
defp do_numeral(n) when n >= 1, do: "I" <> do_numeral(n - 1)
defp do_numeral(0), do: ""
end
``````

@nobbz’s solution is also using recursion. Everything is recursion, `Enum.reduce/3` just makes it look a little like iteration.

3 Likes

FWIW, here is a generic solution that takes a `charlist` as a parameter (might be longer than `'IVXLCDM'`, it only needs to follow the Roman numerals convention) and converts numbers without intermediate checks for “non-round” numerals.

``````defmodule RomanNumerals do
require Integer
@romans 'IVXLCDM'

@spec numeral(pos_integer()) :: String.t()
def numeral(number, romans \\ @romans) do
fours = fn
<<c, c, c, c>>, [last], _f -> <<c, last>>
<<c, c, c, c>>, [c, next | _], _f -> <<c, next>>
<<next, c, c, c, c>>, [c, next, result | _], _f -> <<c, result>>
<<some, c, c, c, c>>, [c, next | _], _f -> <<some, c, next>>
input, [_ | next], f -> f.(input, next, f)
end

romans
|> Enum.with_index()
|> Enum.reverse()
|> Enum.reduce({number, []}, fn {c, i}, {number, acc} ->
denominator =
round(
if Integer.is_even(i),
do: :math.pow(10, i / 2),
else: 5 * :math.pow(10, (i - 1) / 2)
)

{
rem(number, denominator),
acc ++ List.duplicate(c, div(number, denominator))
}
end)
|> elem(1)
|> to_string()
|> String.replace(~r/.?(.)\1\1\1/, fn m -> fours.(m, romans, fours) end)
end
end
``````