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

Questions / Help
,
1

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
2

Elegant

3

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

4 Likes
4

Nice catch!

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

3 Likes
5

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
6

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.

7

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?

8

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

1 Like
9

There is Range.t/2 for that.

1 Like
10

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.

11

You could write a macro :innocent: 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 )

12

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
13

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
14

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

3 Likes
16

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