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

ryanzidago

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

14 comments

#exercism #katas

6 1916 14

2020-04-23 07:20:47 UTC

Showing Posts 1 to 10

Show Best Posts
Show All (oldest first)
Show All (newest first)

collegeimprovements

Elegant

Post #1

kokolegorille

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

Post #2

ryanzidago

Nice catch!

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

Post #3

NobbZ

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.

Post #4

ryanzidago

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.

Post #5

ryanzidago

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?

Post #6

LostKobrakai

This already is how you‘d do that in a typespec: Typespecs reference — Elixir v1.20.2

Post #7

NobbZ

There is Range.t/2 for that.

Post #8

lud

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.

Post #9

sfusato

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).