tio407

tio407

I have a hard time following the code at defp numeral function. Is this well written Elixir code or is it better to use pipes?

defmodule Roman do
  @numerals [M: 1000, CM: 900, D: 500, CD: 400, C: 100, XC: 90, L: 50, XL: 40, X: 10, IX: 9, V: 5, IV: 4, I: 1]

  @doc """
  Convert the number to a roman number.
  """
  @spec numerals(pos_integer) :: String.t()
  def numerals(number) do
    numeral([], number, @numerals)
  end

  defp numeral(curr_nums, number, [{letter, digit}|_t] = num_list) when number >= digit do
    numeral([letter | curr_nums], number - digit, num_list)
  end

  defp numeral(curr_nums, number, [_h|t]) do
    numeral(curr_nums, number, t)
  end

  defp numeral(numerals, _n, []) do
    numerals
      |> Enum.reverse
      |> Enum.map_join("", &to_string/1)
  end
end

What are some suggestions for re-writing this in a better way? It’s my old code but I no longer understand what is happening.

Showing Posts 1 to 3

kip

kip

ex_cldr Core Team

I think its fine except the variable naming makes it a bit hard to see what’s happening. And the odd comment helps. Personally I think a recursive function like this is a good fit for the problem. Perhaps like this:

defmodule Roman do
  @roman_to_decimal_mappings [M: 1000, CM: 900, D: 500, CD: 400, C: 100, XC: 90, L: 50, XL: 40, X: 10, IX: 9, V: 5, IV: 4, I: 1]

  @doc """
  Convert the number to a roman number.
  """
  @spec numerals(pos_integer) :: String.t()
  def numerals(number) do
    numeral([], number, @roman_to_decimal_mappings)
  end

  # While the number is >= the decimal value at the head of the mappings,
  # accumulate the Roman numeral and decrement the number
  defp numeral(roman_acc, number, [{roman_numeral, decimal_value}| _t] = roman_to_decimal_mappings) 
      when number >= decimal_value do
    numeral([roman_numeral | roman_acc], number - decimal_value, roman_to_decimal_mappings)
  end

  # When the number is less than the decimal value at the head of the
  # mapping list, move to the next mapping pair
  defp numeral(roman_acc, number, [_h | remaining_mappings]) do
    numeral(roman_acc, number, remaining_mappings)
  end

  # And when the mapping list is exhausted, reverse the accumulated
  # Roman numerals and join then
  defp numeral(roman_acc, _n, [] = _remaining_mappings) do
    roman_acc
      |> Enum.reverse
      |> Enum.map_join("", &to_string/1)
  end
end
gregvaughn

gregvaughn

I’m not certain how readable this is for others, but it’s how I worked through the problem early in my Elixir adventures, and I can still explain the approach 6 years later. elixir_kata/roman/roman_numerals.exs at master · gvaughn/elixir_kata · GitHub

I iterate over the mappings and use Enum.reduce with a 2-tuple to keep track of the remainder of the number (after dividing out each Roman value) plus the accumulated Roman numeral string.

collegeimprovements

collegeimprovements

defmodule RomanNumerals do
  @spec to_roman(integer) :: String.t()
  def to_roman(number) when is_integer(number) and number > 0 do
    to_roman_recursive(number, "")
  end
  def to_roman(_), do: ""

  defp to_roman_recursive(0, acc), do: acc
  defp to_roman_recursive(number, acc) when number >= 1000, do: to_roman_recursive(number - 1000, acc <> "M")
  defp to_roman_recursive(number, acc) when number >= 900, do: to_roman_recursive(number - 900, acc <> "CM")
  defp to_roman_recursive(number, acc) when number >= 500, do: to_roman_recursive(number - 500, acc <> "D")
  defp to_roman_recursive(number, acc) when number >= 400, do: to_roman_recursive(number - 400, acc <> "CD")
  defp to_roman_recursive(number, acc) when number >= 100, do: to_roman_recursive(number - 100, acc <> "C")
  defp to_roman_recursive(number, acc) when number >= 90, do: to_roman_recursive(number - 90, acc <> "XC")
  defp to_roman_recursive(number, acc) when number >= 50, do: to_roman_recursive(number - 50, acc <> "L")
  defp to_roman_recursive(number, acc) when number >= 40, do: to_roman_recursive(number - 40, acc <> "XL")
  defp to_roman_recursive(number, acc) when number >= 10, do: to_roman_recursive(number - 10, acc <> "X")
  defp to_roman_recursive(number, acc) when number >= 9, do: to_roman_recursive(number - 9, acc <> "IX")
  defp to_roman_recursive(number, acc) when number >= 5, do: to_roman_recursive(number - 5, acc <> "V")
  defp to_roman_recursive(number, acc) when number >= 4, do: to_roman_recursive(number - 4, acc <> "IV")
  defp to_roman_recursive(number, acc) when number >= 1, do: to_roman_recursive(number - 1, acc <> "I")
end
— All posts loaded —

Where Next? Top

Trending in Questions Top

katta
I having some trouble figuring out if I have set myself too strict of standards for my production server. Currently I can handle 75% of r...
New
brecabral
Documentation While reading the Scoped Routes section, I noticed that the documentation currently refers to a problem without explainin...
New
nseaSeb
Hello, I know there is an approach for handling lists that allows for optimized traversal, but I can’t recall the specific method (somet...
New
kpanic
Hi everyone, I am toying with the idea of building a “match maker” for giving personal help to people that wants to start coding. I sta...
New
velrest
So my question is quite simple and i have found no conclusive answer on forum, google or AI. Should we use :erlang.float for Integer to ...
New
asweet-confluent
I recently noticed that Elixir’s Logger defaults its primary log level to :debug when no :logger, :level application configuration is pre...
New
apz
I’m new to elixir and just tried to install the elixirLS extension for VScode(ium) and it is throwing some errors that I would like help ...
New

Other Trending Topics Top

GenericJam
Edit: 2026 May 15 - This post is archived. Mob is alive!! Main docs: mob v0.7.11 — Documentation A bit of explanation for the slightly c...
New
JesseHerrick
Hey, I’m Jesse and I’m the main contributor behind Dexter, a full-featured, lightning-fast Elixir LSP optimized for large codebases. It s...
New
mudasobwa
I am happy to introduce the very α version of the new programming language compiled to BEAM. Welcome Cure. It has literally three kille...
New
garrison
Hobbes is a low-level distributed database for the Elixir programming language. Hobbes provides a simple, safe, and scalable storage lay...
New
mhanberg
Hi everyone! The first release candidate for the Expert language server project is now available! We’ve published a press release detai...
New
budgie
A little off-topic, but I feel like people here have a good head on their shoulders. I used to be quite good at making software. Was luc...
New

We're in Beta

About us Mission Statement

Options

Thread Display Mode




Thread Preview

Skip Thread Previews